DO NOT RETURN "null" try { do_some_stuff(); } catch (Exception e) { return null; }

We will be on the lookout for anything like this when performing a code review.

Stop

Stop returning null! It sounds easy - and it is. Use one of the many "NullObjects" available to you as a stand in. You are expecting the person calling you to do something when you retuned null right? A NullObject is set up to capture exactly what you expect them to do.

Stop eating exceptions!! Exceptions contains valuable informations about what may have gone wrong. Eating all exceptions is especially bad. Catch only the most specific exceptions you are expecting.

In the bad example above, the exception should at least be logged as below:

try {
    do_some_stuff();
}
catch (SomeSpecificException e){
    LOG.warning("Could not do some stuff: " + e);
    return SOME_EMPTY_OBJECT;
}

An alternative is to invokes some convenience unexpectedException method from the org.geotools.util.Logging class. The examples below use the latest approach.

try {
    return a_set;
}
catch (SomeSpecificException e){
    Logging.unexpectedException(LOG, e);
    return Collections.EMPTY_LIST;
}

try {
    return a_expression;
}
catch (SomeSpecificException e){
    Logging.unexpectedException(LOG, e);
    return Expression.NULL;
}

// Often used if it a programmer error encountering a limitation (say SQL encoding)
try {
    return a_filter;
}
catch (SomeSpecificException e){
    Logging.unexpectedException(LOG, e);
    return Expression.EXCLUDES;
}

// Often used if it is a "data problem", return too much data rather then none
try {
    return a_filter;
}
catch (SomeSpecificException e){
    Logging.unexpectedException(LOG, e);
    return Expression.INCLUDES;
}

If you need additional "NullObjects" email the devel list and we will make them happen. We probably should have a NullDataStore and a NullFeatureSource for example ... or throw an Exception.

Drop

Drop the pretense that everything is okay, if you are in a situtation where you are returning null chances are there is a real problem around ... so throw some kind of Exception. This should be used when you have defined your limitations in the javadocs, and the programmer has neglected to read them.

This may require use of runtime exceptions, or the interface you are implementing may already provide a checked exception for you to throw.

try {
    do_some_stuff();
} catch (SomeSpecificException e) {
    return (IOException) new IOException("Could not do some stuff (did you read the javadocs?): "+e).initCause(e);
}

Roll

As a last possible effort; Roll with the flow (but be sure to "log the cause" so client code can notice something is wrong):

try {
    do_some_stuff();
} catch (SomeSpecificException e) {
    Logging.unexpectedException(LOG, e);
    return null;
}

Be sure to use the warning level (we are still making an assumption) - and client code should know about it). And do not worry about filling up the test case results with garbage; testing code that expected to produce warnings happens all the time (the test case code can change the logging level to "Info" and then set it back).