So what exactly *is* the intention of this method? [...]
1.
My intention was only to uniform the codebase around the helper methods that were presumably created for this sort of task.
Yes, I was really addressing other committers generally, sorry for the confusion.
I should add that I don't think this issue is especially important relative to other things, it just caught my eye and I wondered if others cared.
3.try { OutputStream os = new FileOutputStream(...); try { printStuffToStream(os); } finally { os.close(); } } catch (IOException e) { throw new BuildException(e); }
when does finally get called? After the inner try, or at the end of the outer try block?
After the inner try, of course - just considering
OutputStream os = new FileOutputStream(...);
try {
printStuffToStream(os);
} finally {
os.close();
}in isolation, it is clear that 'os' is always closed.
Reading the spec only says that the VM honours the fact that finally will always be called before returning back up the call tree. Certain VM's could handle this differently (although I'm pretty sure they'd all just execute the inner stuff first).
http://java.sun.com/docs/books/jls/second_edition/html/statements.doc.html#236653
"If execution of the try block completes abruptly because of a throw of a value V [....] If the run-time type of V is not assignable to the parameter of any catch clause of the try statement [which must be true since there are no catch clauses in the inner try], then the finally block is executed. Then there is a choice:
* If the finally block completes normally, then the try statement completes abruptly because of a throw of the value V.
* If the finally block completes abruptly for reason S, then the try statement completes abruptly for reason S (and the throw of value V is discarded and forgotten)."
Note that an exception in finally "shadows" an exception in the main block, but I doubt it matters much - probably you want to resolve *all* exceptions so you could start with any of them.
4.
The utility method is designed to swallow the exception and not report it.
I know, that's the problem.
So the rational[e] of these changes is that you care about catching IOExceptions in the general body of the code (FileNotFound etc etc), but you don't want to report an exception when you're trying to clean up in the finally method, as any exceptions would have already been reported and indeed the stream etc would probably have been closed anyway, the finally is simply used as a sanity check.
My point is that if .close() throws an IOException, even if the main body ran OK, something more serious could be wrong. Perhaps a final flush to disk failed, for example. The current Ant code is basically assuming that .close() doesn't do anything interesting. Maybe it doesn't in practice, I don't know, but wouldn't it be safer to report such problems? (And write code to selectively recover from them with only a warning, if we can be sure that the user's requested action has in fact completed.)
I guess .close() on InputStream is less relevant than on OutputStream.
private Class findClassInComponents(String name) throws ClassNotFoundException { [...] InputStream stream = null; [...] try { [...] } finally { try { if (stream != null) { stream.close(); } } catch (IOException e) { //ignore } } }
Here the logging of any exceptions happens as normal and then in the finally we just want to make sure that the stream is closed. Unfortunately as the close method throws IOException, we have to have yet another try/catch when we don't care about the exception at all.
But if you wrap the entire thing in one try-catch for IOException, there is not really any extra code needed.
Is it a good thing to simply swallow exceptions? [...]
Well it is often harmless, and of course if you know for sure why the exception might be thrown and that it is not a truly exceptional situation, then fine. But once in a while some rare exception - "write failed, disk full" - is critical to understanding why a whole program failed to work quite right. If you get sources for the code that is swallowing the exception, your best bet is to patch it to use printStackTrace(), and that is really a pain. I have wasted hours trying to figure out why a user of my code was occasionally seeing ClassLoader.getResourceAsStream(...) return null in an odd place, until I put in some heavy debugging code and found that getResource(...) was fine but opening the stream failed with a stupid problem that was swallowed but easily corrected once seen.
I can see where your'e going with this. You use the catch to wrap everything, and don't catch locally. The only problem is where you want to log the exact message at the time of the exception do you lose the ability when you catch at the end?
Exception.getMessage() you mean? No, that's fine. Of course, there are cases you where you really want to catch some exceptions early, log them, and try to continue with something else... but it's not so common.
Also I like the way you don't check for nulls - is this NullPointerException safe? if 'is' is null when you call close in finally (guaranteed to happen), [...]
InputStream is = new FileInputStream(...); try { [any code] } finally { is.close(); }
'is' cannot be null, unless of course you set it to null in the try block.
Also note that there is no need to wrap the FileInputStream block in 'finally': either it succeeds, in which case you continue to the 'try' block, or it fails with an exception, in which case there is nothing to close. You can't put anything between the constructor and the try block, e.g.
Reader r = new WhateverReader(new FileInputStream(...));
try {
[any code]
} finally {
r.close();
}is not safe - use
InputStream is = new FileInputStream(...);
try {
Reader r = new WhateverReader(is);
[any code]
} finally {
is.close();
}-J.
-- Jesse Glick <mailto:[EMAIL PROTECTED]> x22801 NetBeans, Open APIs <http://www.netbeans.org/>
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
