mimaison commented on code in PR #13201:
URL: https://github.com/apache/kafka/pull/13201#discussion_r1360412557


##########
tools/src/main/java/org/apache/kafka/tools/ToolsUtils.java:
##########
@@ -123,4 +125,21 @@ public static void validateBootstrapServer(String 
hostPort) throws IllegalArgume
             throw new IllegalArgumentException("Please provide valid host:port 
like host1:9091,host2:9092\n");
         }
     }
+
+    public static <T> List<T> duplicates(List<T> s) {

Review Comment:
   Should the return type be `Set` instead of `List` as each entry in the 
returned value should only appear once?
   If so this could be simplified into something like, so we only iterate the 
input list once:
   ```
   Set<T> set = new HashSet<>();
   Set<T> duplicates = new HashSet<>();
   s.forEach(element -> {
       if (!set.add(element)) {
           duplicates.add(element);
       }
   });
   return duplicates;
   ```
   Could we also add a short javadoc explaining the return value only contains 
each duplicate value once?



##########
core/src/test/scala/unit/kafka/utils/TestUtils.scala:
##########
@@ -2395,6 +2395,8 @@ object TestUtils extends Logging {
     )
 
     def unexpectedThreads: Set[String] = {
+      val x = Thread.getAllStackTraces.asScala.filter(entry => 
unexpectedThreadNames.exists(s => entry._1.getName.contains(s)))

Review Comment:
   I guess it's a leftover from some debugging?



##########
tools/src/main/java/org/apache/kafka/tools/TopicCommand.java:
##########
@@ -654,7 +651,7 @@ public List<String> getTopics(Optional<String> 
topicIncludeList, boolean exclude
         public List<Uuid> getTopicIds(Uuid topicIdIncludeList, boolean 
excludeInternalTopics) throws ExecutionException, InterruptedException {
             ListTopicsResult allTopics = excludeInternalTopics ? 
adminClient.listTopics() :
                 adminClient.listTopics(new 
ListTopicsOptions().listInternal(true));
-            List<Uuid> allTopicIds = null;
+            List<Uuid> allTopicIds;

Review Comment:
   Since we assign it just below, could we just do:
   ```
   List<Uuid> allTopicIds = allTopics.listings().get().stream()
           .map(TopicListing::topicId)
           .sorted()
   ```?



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

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to