[GitHub] [accumulo] lbschanno commented on a change in pull request #1676: WIP - convert translators to lambdas

[GitBox <gi...@apache.org>]

lbschanno commented on a change in pull request #1676:
URL: https://github.com/apache/accumulo/pull/1676#discussion_r468681380



##########
File path: core/src/main/java/org/apache/accumulo/core/clientImpl/Translator.java
##########
@@ -41,13 +41,17 @@
   public abstract OT translate(IT input);
 
   public static class TKeyExtentTranslator extends Translator<TKeyExtent,KeyExtent> {
+
     @Override
     public KeyExtent translate(TKeyExtent input) {
       return new KeyExtent(input);
     }
-
   }
 
+

Review comment:
       Please remove these empty lines.

##########
File path: core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchReaderIterator.java
##########
@@ -533,15 +535,19 @@ static void trackScanning(Map<KeyExtent,List<Range>> failures,
       Map<KeyExtent,List<Range>> unscanned, MultiScanResult scanResult) {
 
     // translate returned failures, remove them from unscanned, and add them to failures
-    Map<KeyExtent,List<Range>> retFailures = Translator.translate(scanResult.failures,
-        Translators.TKET, new Translator.ListTranslator<>(Translators.TRT));
+         Map<KeyExtent, List<Range>> retFailures = scanResult.failures.entrySet().stream()
+            .collect(Collectors.toMap(
+                    entry -> new KeyExtent(entry.getKey()),
+                    entry -> entry.getValue().stream().map(Range::new).collect(Collectors.toList())
+          ));
+
     unscanned.keySet().removeAll(retFailures.keySet());
     failures.putAll(retFailures);
 
-    // translate full scans and remove them from unscanned
-    HashSet<KeyExtent> fullScans =
-        new HashSet<>(Translator.translate(scanResult.fullScans, Translators.TKET));
-    unscanned.keySet().removeAll(fullScans);
+        // translate full scans and remove them from unscanned
+
+    Set<KeyExtent> fullScans = scanResult.fullScans.stream().map(KeyExtent::new).collect(Collectors.toSet());
+        unscanned.keySet().removeAll(fullScans);

Review comment:
       Please remove the indent here.

##########
File path: core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchReaderIterator.java
##########
@@ -533,15 +535,19 @@ static void trackScanning(Map<KeyExtent,List<Range>> failures,
       Map<KeyExtent,List<Range>> unscanned, MultiScanResult scanResult) {
 
     // translate returned failures, remove them from unscanned, and add them to failures
-    Map<KeyExtent,List<Range>> retFailures = Translator.translate(scanResult.failures,
-        Translators.TKET, new Translator.ListTranslator<>(Translators.TRT));
+         Map<KeyExtent, List<Range>> retFailures = scanResult.failures.entrySet().stream()

Review comment:
       We've already discussed this in person, but I'm including a comment here so anyone else can chime in; these kinds of transformation operations are good candidates for being moved to a separate utility class that supports multiple types of collection transformations using lambdas and function parameters. This would allow shorter function calls to improve readability, as well as make it available to the project. I'd be happy to help out with this effort.

##########
File path: core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchReaderIterator.java
##########
@@ -533,15 +535,19 @@ static void trackScanning(Map<KeyExtent,List<Range>> failures,
       Map<KeyExtent,List<Range>> unscanned, MultiScanResult scanResult) {
 
     // translate returned failures, remove them from unscanned, and add them to failures
-    Map<KeyExtent,List<Range>> retFailures = Translator.translate(scanResult.failures,
-        Translators.TKET, new Translator.ListTranslator<>(Translators.TRT));
+         Map<KeyExtent, List<Range>> retFailures = scanResult.failures.entrySet().stream()
+            .collect(Collectors.toMap(
+                    entry -> new KeyExtent(entry.getKey()),
+                    entry -> entry.getValue().stream().map(Range::new).collect(Collectors.toList())
+          ));
+
     unscanned.keySet().removeAll(retFailures.keySet());
     failures.putAll(retFailures);
 
-    // translate full scans and remove them from unscanned
-    HashSet<KeyExtent> fullScans =
-        new HashSet<>(Translator.translate(scanResult.fullScans, Translators.TKET));
-    unscanned.keySet().removeAll(fullScans);
+        // translate full scans and remove them from unscanned

Review comment:
       Can you move this comment one line down with the indent removed?




----------------------------------------------------------------
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