New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

squid:S2095 - Resources should be closed #516

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@georgekankava
Contributor

georgekankava commented Feb 18, 2016

This pull request is focused on resolving occurrences of Sonar rules
squid:S2095 - Resources should be closed.
You can find more information about the issue here:
https://dev.eclipse.org/sonar/coding_rules#q=squid%3AS2095
Please let me know if you have any questions.
George Kankava

@codecov-io

This comment has been minimized.

codecov-io commented Feb 18, 2016

Current coverage is 57.49%

Merging #516 into master will increase coverage by +0.01% as of 4d036f4

@@            master    #516   diff @@
======================================
  Files          143     143       
  Stmts        15137   15139     +2
  Branches      2966    2968     +2
  Methods          0       0       
======================================
+ Hit           8701    8704     +3
- Partial       1176    1177     +1
+ Missed        5260    5258     -2

Review entire Coverage Diff as of 4d036f4

Powered by Codecov. Updated on successful CI builds.

@vlsi

This comment has been minimized.

Member

vlsi commented Feb 18, 2016

Technically speaking, statement.close closes the ResultSet as well.

Regarding the PR: I would prefer adding try-finally to close the statement rather than adding rs.close there.
The real issue is "finally not being used".

@georgekankava georgekankava force-pushed the georgekankava:release/resources-should-be-closed-fix-1 branch from 3e091bd to 784dbb0 Feb 18, 2016

@georgekankava

This comment has been minimized.

Contributor

georgekankava commented Feb 18, 2016

Please take a look at this change.

stmt = connection.createStatement();
ResultSet rs = stmt.executeQuery(sql);
if (!rs.next()) {
stmt.close();

This comment has been minimized.

@vlsi

vlsi Feb 18, 2016

Member

please remove this explicit close (it would be closed in finally)

@georgekankava georgekankava force-pushed the georgekankava:release/resources-should-be-closed-fix-1 branch from 784dbb0 to 7593faf Feb 18, 2016

@georgekankava

This comment has been minimized.

Contributor

georgekankava commented Feb 18, 2016

That's correct. Thank you. Done

@georgekankava georgekankava force-pushed the georgekankava:release/resources-should-be-closed-fix-1 branch from 7593faf to e234efa Feb 19, 2016

@georgekankava georgekankava force-pushed the georgekankava:release/resources-should-be-closed-fix-1 branch from e234efa to 2900097 Feb 19, 2016

@vlsi vlsi closed this in b4c45ca Jun 22, 2016

zemian pushed a commit to zemian/pgjdbc that referenced this pull request Oct 6, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment