Maybe this belongs in another forum; sorry if I picked the wrong one. This is a DAO. This is the original code that FindBugs reported may fail to close database objects.
I see lots of problems with this code but most important is the lack of a finally block. I won't even go into the use of Vector, hard coded database credentials (removed to protect the guilty) and concatenating Strings inside a loop. My main concern is leaving database resources open. I'll fix the other issues later. For now, I changed it to this:
This is modeled after other DAOs I've written which FindBugs doesn't complain about. However, Findbugs is telling me that this code may still fail to close a database resource. It looks to me like the finally block should close everything no matter how the method exits. What am I missing?
"There is no reason for any individual to have a computer in his home" ~ Ken Olson, Co-founder of DEC, 1977
Yes, that code is fine. The only way one of those close() calls wouldn't be reached is if an Error is thrown in a previous close(), like OutOfMemoryError. You could catch Throwable instead of Exception inside the finally to handle that case, but I wouldn't advise that. If a RuntimeException is thrown, that's one thing. A coding error inside a close() shouldn't prevent a later close(). But an Error is a different story. That's more severe, usually indicative of a serious problem inside the JVM. Generally best just to let those bubble all the way up and kill the JVM. And if you do catch Error, you'd have to rethrow ThreadDeath.
It might be interesting to catch Error just to see if that shuts findbugs up, or if it's just confused about something else.
Finally, note that if you're in Java 7, you can use "with resources" or something like that to automatically close stuff that you explicitly mention at the start of the "with" block. Not sure of the details, but you an google for it if that's an option for you.
Thanks for checking it for me. I did a clean and build and recreated the findbugs project and it's no longer complaining. It's a useful tool, but it can be rather quirky.
Interesting tactic, catching Error or Throwable instead of Exception. I'll keep that one in mind in the future. I realize it's not a good idea for production code but it could be very useful for debugging.