You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2019/12/25 20:49:44 UTC
[GitHub] [commons-csv] 0x100 opened a new pull request #52: Add
CSVRecord.isSet(int) method
0x100 opened a new pull request #52: Add CSVRecord.isSet(int) method
URL: https://github.com/apache/commons-csv/pull/52
Added the isSet(int) method to CSVRecord for checking whether a column with a given index is set. Also removed unnecessary unboxing of an Integer variable in the CSVRecord#isSet(String) method.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-csv] garydgregory merged pull request #52: Add
CSVRecord.isSet(int) method
Posted by GitBox <gi...@apache.org>.
garydgregory merged pull request #52: Add CSVRecord.isSet(int) method
URL: https://github.com/apache/commons-csv/pull/52
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-csv] garydgregory commented on a change in pull request
#52: Add CSVRecord.isSet(int) method
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #52: Add CSVRecord.isSet(int) method
URL: https://github.com/apache/commons-csv/pull/52#discussion_r361339000
##########
File path: src/main/java/org/apache/commons/csv/CSVRecord.java
##########
@@ -211,7 +211,18 @@ public boolean isMapped(final String name) {
* @return whether a given columns is mapped and has a value
*/
public boolean isSet(final String name) {
- return isMapped(name) && getHeaderMapRaw().get(name).intValue() < values.length;
+ return isMapped(name) && getHeaderMapRaw().get(name) < values.length;
Review comment:
Let's keep the boxing as is please and focus this PR on the new API.
If you want to go through the whole code base and consistently use one boxing style that's a different PR IMO. But: I prefer to know when boxing and unboxing takes place since it is not free; this gives the compiler the opportunity to flag unintentional boxing and unboxing and let the programmer decide that's really what should happen.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-csv] coveralls commented on issue #52: Add
CSVRecord.isSet(int) method
Posted by GitBox <gi...@apache.org>.
coveralls commented on issue #52: Add CSVRecord.isSet(int) method
URL: https://github.com/apache/commons-csv/pull/52#issuecomment-568931193
[](https://coveralls.io/builds/27792369)
Coverage increased (+0.007%) to 92.928% when pulling **909b19e807afc363983b6a9c419f566ca4c0f54b on 0x100:isset_i** into **129641ead46c8faefdb18903e283f7c4f55a000d on apache:master**.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-csv] coveralls edited a comment on issue #52: Add
CSVRecord.isSet(int) method
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #52: Add CSVRecord.isSet(int) method
URL: https://github.com/apache/commons-csv/pull/52#issuecomment-568931193
[](https://coveralls.io/builds/27792687)
Coverage increased (+0.007%) to 92.928% when pulling **827565a715def081359f6d01c782437bdf92b8ff on 0x100:isset_i** into **129641ead46c8faefdb18903e283f7c4f55a000d on apache:master**.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [commons-csv] 0x100 commented on a change in pull request #52: Add
CSVRecord.isSet(int) method
Posted by GitBox <gi...@apache.org>.
0x100 commented on a change in pull request #52: Add CSVRecord.isSet(int) method
URL: https://github.com/apache/commons-csv/pull/52#discussion_r361339413
##########
File path: src/main/java/org/apache/commons/csv/CSVRecord.java
##########
@@ -211,7 +211,18 @@ public boolean isMapped(final String name) {
* @return whether a given columns is mapped and has a value
*/
public boolean isSet(final String name) {
- return isMapped(name) && getHeaderMapRaw().get(name).intValue() < values.length;
+ return isMapped(name) && getHeaderMapRaw().get(name) < values.length;
Review comment:
Ok, I've rolled back this change.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services