commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: [imaging] Closing stream
Date Thu, 24 Oct 2013 02:00:33 GMT
On 24 October 2013 02:49, Gary Gregory <garydgregory@gmail.com> wrote:
> On Wed, Oct 23, 2013 at 9:38 PM, sebb <sebbaz@gmail.com> wrote:
>
>> On 24 October 2013 01:16, Gary Gregory <garydgregory@gmail.com> 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: garydgregory@gmail.com | ggregory@apache.org
>> > 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: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@apache.org
> 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: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message