ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jesse Glick <jesse.gl...@sun.com>
Subject Purpose of FileUtils.close(...) (was: Re: [Patch] FileUtils more minor changes)
Date Thu, 16 Dec 2004 21:20:20 GMT
Kev Jackson wrote:
> As an aside, I went through the entire codebase last night and replaced 
> every
> try {
>  somthing.close()
> } catch (IOException e) {
>    //swallow exception
> }
> 
> with FileUtils.close()

So what exactly *is* the intention of this method? IMHO, if an 
IOException was thrown when some stream was closed, it was probably for 
a reason, and probably something is broken, and probably the user should 
find out about it - and fix it - rather than have the problem be 
permanently suppressed from view. Maybe an exception closing a stream is 
harmless, but Ant surely doesn't know this; it could mean the last few 
Kb did not get flushed to disk, for example. E.g. I often see in Ant 
code like this (roughly adopted from ChangeLogTask.writeChangelog):

OutputStream os = null;
try {
     os = new FileOutputStream(...);
     printStuffToStream(os);
} catch (IOException e) {
     throw new BuildException(e);
} finally {
     FileUtils.close(os);
}

To my mind this is both harder to follow the flow of, and less robust 
than, the straightforward version:

try {
     OutputStream os = new FileOutputStream(...);
     try {
         printStuffToStream(os);
     } finally {
         os.close();
     }
} catch (IOException e) {
     throw new BuildException(e);
}

Here it is obvious than any IOException's get reported, and that an 
OutputStream will always be closed if it ever got created. You don't 
need any special utility method, or any dummy initial null value. You 
can even make the local var final if that floats your boat.

When there is more than one thing to be closed - e.g. an InputStream and 
an OutputStream - using nested try-finally blocks makes it clearer that 
everything gets closed, with no "if (os != null) {....}" weirdness:

try {
     InputStream is = new FileInputStream(...);
     try {
         OutputStream os = new FileOutputStream(...);
         try {
             copyStuff(is, os);
         } finally {
             os.close();
         }
     } finally {
         is.close();
     }
} catch (IOException e) {
     throw new BuildException(e);
}

Thoughts?

-J.

-- 
Jesse Glick <mailto:jesse.glick@sun.com> x22801
NetBeans, Open APIs  <http://www.netbeans.org/>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Mime
View raw message