On 24 October 2013 02:49, Gary Gregory <[email protected]> wrote:
> On Wed, Oct 23, 2013 at 9:38 PM, sebb <[email protected]> wrote:
>
>> On 24 October 2013 01:16, Gary Gregory <[email protected]> wrote:
>> > Hi All:
>> >
>> > I see a log of this pattern:
>> >
>> > try {
>> > if (outputStream != null) {
>> > outputStream.close();
>> > }
>> > } catch (final Exception e) {
>> > Debug.debug(e);
>> > }
>> >
>> > for example in org.apache.commons.imaging.Imaging:
>> >
>> > public static void writeImage(final BufferedImage src, final File
>> file,
>> > final ImageFormat format, final Map<String,Object> params)
>> > throws ImageWriteException,
>> > IOException {
>> > OutputStream os = null;
>> >
>> > try {
>> > os = new FileOutputStream(file);
>> > os = new BufferedOutputStream(os);
>> >
>> > writeImage(src, os, format, params);
>> > } finally {
>> > try {
>> > if (os != null) {
>> > os.close();
>> > }
>> > } catch (final Exception e) {
>> > Debug.debug(e);
>> > }
>> > }
>> > }
>> >
>> > Which seems wrong to me. If I get an IOE while writing, we throw, but if
>> we
>> > get one when flushing and closing the file (which may also write), we
>> > swallow it. Why? It seems the IOE from close should percolate up and not
>> be
>> > swallowed.
>>
>> I agree that's bad practice - not only because of swallowing the Exception.
>> The catch block should catch IOException not Exception.
>>
>
> I do not think we should even catch the IOE, it should percolate up, just
> like the writeImage call. Why not simply:
>
> {
> final OutputStream os = new BufferedOutputStream(new
> FileOutputStream(file));
> try {
> writeImage(src, os, format, params);
> } finally {
> os.close();
> }
> }
OK by me.
I don't like the Debug.debug(e) statements; does not see appropriate
for a library.
> Or if your are really paranoid:
>
> {
> OutputStream os = new FileOutputStream(file);
> try {
> os = new BufferedOutputStream(os);
I don't like the reuse of the "os" variable here.
> writeImage(src, os, format, params);
> } finally {
> os.close();
> }
> }
>
> I'm looking for a pattern we can apply throughout [imaging] before 1.0.
>
> Gary
>
>
>> > All of this is moot in Java 7 with try-with-resources blocks but we are
>> not
>> > ready for Java 7 here I imagine.
>> >
>> > Gary
>> > --
>> > E-Mail: [email protected] | [email protected]
>> > Java Persistence with Hibernate, Second Edition<
>> http://www.manning.com/bauer3/>
>> > JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>> > Spring Batch in Action <http://www.manning.com/templier/>
>> > Blog: http://garygregory.wordpress.com
>> > Home: http://garygregory.com/
>> > Tweet! http://twitter.com/GaryGregory
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [email protected]
>> For additional commands, e-mail: [email protected]
>>
>>
>
>
> --
> E-Mail: [email protected] | [email protected]
> Java Persistence with Hibernate, Second
> Edition<http://www.manning.com/bauer3/>
> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> Spring Batch in Action <http://www.manning.com/templier/>
> Blog: http://garygregory.wordpress.com
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
