You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2020/04/24 14:36:15 UTC
[GitHub] [accumulo] jmark99 opened a new pull request #1595: Update manual array to collections copy method.
jmark99 opened a new pull request #1595:
URL: https://github.com/apache/accumulo/pull/1595
Replace instances of copying of array contents to a collection where each element is added individually using a for loop with a call to Collections.addAll().
----------------------------------------------------------------
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
[GitHub] [accumulo] EdColeman commented on a change in pull request #1595: Update manual array to collections copy method.
Posted by GitBox <gi...@apache.org>.
EdColeman commented on a change in pull request #1595:
URL: https://github.com/apache/accumulo/pull/1595#discussion_r414634649
##########
File path: core/src/main/java/org/apache/accumulo/core/data/Key.java
##########
@@ -1159,7 +1159,7 @@ public int getSize() {
}
private static boolean isEqual(byte[] a1, byte[] a2) {
- if (a1 == a2)
+ if (Arrays.equals(a1, a2))
Review comment:
I'm not sure this keeps the same behaviour. The == will test if the passed arrays are the same reference. Replacing with Arrays.equals is also going to check if the contents are the same. This would make the rest of the test redundant - and from the code comments is trying to optimize the comparison.
----------------------------------------------------------------
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
[GitHub] [accumulo] jmark99 commented on a change in pull request #1595: Update manual array to collections copy method.
Posted by GitBox <gi...@apache.org>.
jmark99 commented on a change in pull request #1595:
URL: https://github.com/apache/accumulo/pull/1595#discussion_r414636098
##########
File path: core/src/main/java/org/apache/accumulo/core/data/Key.java
##########
@@ -1159,7 +1159,7 @@ public int getSize() {
}
private static boolean isEqual(byte[] a1, byte[] a2) {
- if (a1 == a2)
+ if (Arrays.equals(a1, a2))
Review comment:
Thanks for the catch Ed! I will revert that change. It didn't really go with the rest of the changes anyway.
----------------------------------------------------------------
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
[GitHub] [accumulo] keith-turner commented on a change in pull request #1595: Update manual array to collections copy method.
Posted by GitBox <gi...@apache.org>.
keith-turner commented on a change in pull request #1595:
URL: https://github.com/apache/accumulo/pull/1595#discussion_r414758175
##########
File path: core/src/main/java/org/apache/accumulo/core/clientImpl/mapreduce/lib/InputConfigurator.java
##########
@@ -360,9 +361,7 @@ public static void fetchColumns(Class<?> implementingClass, Configuration conf,
List<String> serialized = new ArrayList<>();
if (confValue != null) {
// Split and include any trailing empty strings to allow empty column families
- for (String val : confValue.split(",", -1)) {
- serialized.add(val);
- }
+ Collections.addAll(serialized, confValue.split(",", -1));
Review comment:
This is a handy method, I didn't know it existed.
----------------------------------------------------------------
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