Skip to content
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

Replace manual array copy #500

Merged
merged 1 commit into from
Oct 17, 2017
Merged

Conversation

JamieMagee
Copy link
Member

Replace manual array copying with System.arraycopy().

Replace manual array copying with System.arraycopy().
@codecov-io
Copy link

codecov-io commented Sep 22, 2017

Codecov Report

Merging #500 into dev will decrease coverage by 0.04%.
The diff coverage is 33.33%.

Impacted file tree graph

@@             Coverage Diff             @@
##                dev    #500      +/-   ##
===========================================
- Coverage     46.64%   46.6%   -0.05%     
+ Complexity     2218    2216       -2     
===========================================
  Files           108     108              
  Lines         25279   25276       -3     
  Branches       4167    4164       -3     
===========================================
- Hits          11791   11779      -12     
  Misses        11463   11463              
- Partials       2025    2034       +9
Flag Coverage Δ Complexity Δ
#JDBC41 46.37% <33.33%> (-0.04%) 2200 <0> (-5)
#JDBC42 46.42% <33.33%> (-0.05%) 2210 <0> (-3)
Impacted Files Coverage Δ Complexity Δ
...rc/main/java/com/microsoft/sqlserver/jdbc/DDC.java 44.92% <0%> (+1.67%) 105 <0> (+1) ⬆️
...oft/sqlserver/jdbc/SQLServerPreparedStatement.java 51.57% <50%> (ø) 164 <0> (-1) ⬇️
...in/java/com/microsoft/sqlserver/jdbc/IOBuffer.java 53.78% <0%> (-0.5%) 0% <0%> (ø)
...c/main/java/com/microsoft/sqlserver/jdbc/Util.java 60.08% <0%> (-0.44%) 89% <0%> (-1%)
...n/java/com/microsoft/sqlserver/jdbc/DataTypes.java 77.62% <0%> (-0.17%) 4% <0%> (-1%)
...om/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java 52.58% <0%> (-0.13%) 239% <0%> (ø)
...m/microsoft/sqlserver/jdbc/SQLServerResultSet.java 32.84% <0%> (-0.13%) 242% <0%> (ø)
...m/microsoft/sqlserver/jdbc/SQLServerStatement.java 59.36% <0%> (-0.11%) 130% <0%> (-1%)
.../microsoft/sqlserver/jdbc/SQLServerConnection.java 45.98% <0%> (-0.04%) 277% <0%> (ø)
...om/microsoft/sqlserver/jdbc/ReaderInputStream.java 44.94% <0%> (ø) 16% <0%> (ø) ⬇️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 24dfd49...0b410f0. Read the comment docs.

@cheenamalhotra cheenamalhotra added this to the 6.3.4 milestone Sep 26, 2017
Copy link
Contributor

@peterbae peterbae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the array loop provides better code readability, and approving this would mean we want to apply this change to other places in the code that uses arrays in this manner. I'm ok with leaving the manual array copy as it is.

@JamieMagee
Copy link
Member Author

@peterbae These changes were detected by FIndBugs as array copying that could be directly replaced by System.arraycopy(). Any other array copy methods contain more advanced logic, and can't be directly replaced by System.arraycopy().

I'm not suggesting changing any of the others, but just these 3 simple instances.

@peterbae peterbae merged commit fba39c8 into microsoft:dev Oct 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants