Details
-
Type:
Improvement
-
Status: Closed
-
Priority:
Minor
-
Resolution: Fixed
-
Affects Version/s: Trunk
-
Fix Version/s: 16.11.01
-
Component/s: ALL COMPONENTS
-
Labels:None
Description
At r1758927 I tried to fixe a bunch of closes missing, reported as warnings in Eclipse
Jacopo noticed and reported on dev ML http://markmail.org/message/b4hvzi7foftfq3j5 2 possible issues:
- you are adding a close statement to code that already had the close statement in the "finally" block; your modification actually introduces a code pattern that is not correct (if an exception is thrown your close statement are not executed)
- I suspect that you are closing the socket connection too early in PcChargeApi
So I reverted r1758927 to check the issues
Activity
- All
- Comments
- Work Log
- History
- Activity
- Transitions
Point 1)
The best option when closing resources (which implement AutoCloseable) is actually to use the try-with-resources statement. At least when there are no performance issues.
I have fixed the issue (only in DatabaseUtil.java) this way because
- there are not performance issues, we anyway create a Statement in both cases, we don't reuse one.
- The finally part is cleaner
I have also used the try-with-resources statement in other places where it fitted.
There is only 1 place where I was undecided: ScriptUtil.parseScript() which uses GroovyUtil.parseClass().
I put only a logError() there and returned null.
I did not throw an IOException because else it spreads everywhere in FlexibleStringExpander.
Here introducing Optional would be better, but I have no time for that today.
Done at revision: 1758951
I completed the point 2 at r1758990, better to close the Socket at the last occasions.
Reopening, there maybe still an issue with the point 2
All is fixed after r1759082 and r1759088
This should be OK now, please reopen if you find an issue.
I see no problem with the point 2, but the point 1 is more complex (I wonder why Eclipse would be wrong) and I'll double check all details...