Some additional minor tips:

Assumptions
-----------
The various ImageWorker.write(...) methods make some assumptions that could be
avoided. They unconditionnaly wrap the given output into an ImageOutputStream
without checking if the output is acceptable as-is (including checking if the
output is already an ImageOutputStream). This check could be done with:

   ImageWriter.getOriginatingProvider().getOutputTypes();

and comparing the returned array with the given output (note: we need to remind
that "originatingProvider" may be null).

ImageWorker.write(...) also reformat inconditionnaly the image without checking
if it is needed for a given ImageWriter. This check could be done with:

    ImageWriter.getOriginatingProvider().canEncodeImage(image);

This is not a 100% reliable test, but still a good one.



Logging exceptions
------------------
In try - catch block, I suggest the avoid the following:

try {
   ...
} catch (whatever exception) {
    LOGGER.log(Level.SEVERE, exception.getLocalizedMessage(), exception);
    throw (IOException) new IOException().initCause(e);
}

I mean, if the exception is rethrown, I don't think that we should log it at
Level.SEVERE (I would said don't log it at all). In many case it cause the
exception to be dumped twice on the console and add confusion since we have the
feeling that the error occured twice. It also add noise to applications that
want to handle that exception.


Logging (other)
---------------
I see some code like:

    if (LOGGER.isLoggable(Level.FINE))
        LOGGER.fine("Getting a JPEG writer and configuring it.");

...but:

1) LOGGER.isLoggable(...) is not needed here. The LOGGER.fine("...") call
   already do this check. "isLoggable" is useful only when the fine("...")
   argument is expensive to compute. In the above case, it is not since we
   just pass a fully formed String. It is likely to just add an (admitly
   very small) overhead.

2) Level.FINE is too high for this kind of logging. By Java Logging convention,
   entering and existing a method are logged at Level.FINER.


        Martin

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
Geotools-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Reply via email to