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 2022/08/10 18:10:13 UTC

[GitHub] [accumulo] EdColeman opened a new pull request, #2870: fix codestyle warnings

EdColeman opened a new pull request, #2870:
URL: https://github.com/apache/accumulo/pull/2870

   These changes should not modify the behavior.  (Noticed the class had a lot of warning while working something else)  
   
   This fixes all but two - wanting to autoclose the terminal printWriter (line 986 - wants a try-with-resources) and an empty if (line 786) but changing the condition removing that seemed make the code more confusing.


-- 
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: notifications-unsubscribe@accumulo.apache.org

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


[GitHub] [accumulo] EdColeman commented on a diff in pull request #2870: fix code style warnings in Shell code

Posted by GitBox <gi...@apache.org>.
EdColeman commented on code in PR #2870:
URL: https://github.com/apache/accumulo/pull/2870#discussion_r943528530


##########
shell/src/main/java/org/apache/accumulo/shell/Shell.java:
##########
@@ -1093,11 +1089,7 @@ public final void printRecords(Iterable<Entry<Key,Value>> scanner, FormatterConf
   }
 
   public static String repeat(String s, int c) {
-    StringBuilder sb = new StringBuilder();
-    for (int i = 0; i < c; i++) {
-      sb.append(s);
-    }
-    return sb.toString();
+    return String.valueOf(s).repeat(Math.max(0, c));

Review Comment:
   Fixed in 15e9cfc223



-- 
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: notifications-unsubscribe@accumulo.apache.org

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


[GitHub] [accumulo] dlmarion commented on a diff in pull request #2870: fix code style warnings in Shell code

Posted by GitBox <gi...@apache.org>.
dlmarion commented on code in PR #2870:
URL: https://github.com/apache/accumulo/pull/2870#discussion_r943386151


##########
shell/src/main/java/org/apache/accumulo/shell/Shell.java:
##########
@@ -1093,11 +1089,7 @@ public final void printRecords(Iterable<Entry<Key,Value>> scanner, FormatterConf
   }
 
   public static String repeat(String s, int c) {
-    StringBuilder sb = new StringBuilder();
-    for (int i = 0; i < c; i++) {
-      sb.append(s);
-    }
-    return sb.toString();
+    return String.valueOf(s).repeat(Math.max(0, c));

Review Comment:
   ```suggestion
       return s.repeat(Math.max(0, c));
   ```



##########
shell/src/main/java/org/apache/accumulo/shell/Shell.java:
##########
@@ -812,23 +808,23 @@ public void execCommand(String input, boolean ignoreAuthTimeout, boolean echoPro
   private ShellCompletor setupCompletion() {
     rootToken = new Token();
 
-    Set<String> tableNames = null;
+    Set<String> tableNames;
     try {
       tableNames = accumuloClient.tableOperations().list();
     } catch (Exception e) {
       log.debug("Unable to obtain list of tables", e);
       tableNames = Collections.emptySet();
     }
 
-    Set<String> userlist = null;
+    Set<String> userlist;

Review Comment:
   IIRC we had to initialize variables to null, and now we don't?



-- 
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: notifications-unsubscribe@accumulo.apache.org

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


[GitHub] [accumulo] EdColeman commented on a diff in pull request #2870: fix code style warnings in Shell code

Posted by GitBox <gi...@apache.org>.
EdColeman commented on code in PR #2870:
URL: https://github.com/apache/accumulo/pull/2870#discussion_r943526511


##########
shell/src/main/java/org/apache/accumulo/shell/Shell.java:
##########
@@ -812,23 +808,23 @@ public void execCommand(String input, boolean ignoreAuthTimeout, boolean echoPro
   private ShellCompletor setupCompletion() {
     rootToken = new Token();
 
-    Set<String> tableNames = null;
+    Set<String> tableNames;
     try {
       tableNames = accumuloClient.tableOperations().list();
     } catch (Exception e) {
       log.debug("Unable to obtain list of tables", e);
       tableNames = Collections.emptySet();
     }
 
-    Set<String> userlist = null;
+    Set<String> userlist;

Review Comment:
   I believe the ide is now smart enough to see that the value is set on all paths before it is used, so the initial set is redundant.  The IDE flags it with 
   ```
   Variable 'userlist' initializer 'null' is redundant
   ```
   
   On the other hand, this will be flagged as an error - `variable n might not have been initialized`
   
   ```
      String n;
       if(n == null){
   
       }
   ```



-- 
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: notifications-unsubscribe@accumulo.apache.org

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


[GitHub] [accumulo] EdColeman merged pull request #2870: fix code style warnings in Shell code

Posted by GitBox <gi...@apache.org>.
EdColeman merged PR #2870:
URL: https://github.com/apache/accumulo/pull/2870


-- 
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: notifications-unsubscribe@accumulo.apache.org

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