[GitHub] [accumulo] ctubbsii commented on a diff in pull request #2695: Minor little cleanups

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

ctubbsii commented on code in PR #2695:
URL: https://github.com/apache/accumulo/pull/2695#discussion_r872507722


##########
core/src/main/java/org/apache/accumulo/core/conf/Property.java:
##########
@@ -1581,76 +1578,35 @@ public static boolean isSensitive(String key) {
     Property prop = propertiesByKey.get(key);
     if (prop != null) {
       return prop.isSensitive();
-    } else {
-      for (String prefix : validPrefixes) {
-        if (key.startsWith(prefix)) {
-          if (propertiesByKey.get(prefix).isSensitive()) {
-            return true;
-          }
-        }
-      }
     }
-    return false;
+    return validPrefixes.stream().filter(key::startsWith).map(propertiesByKey::get)
+        .anyMatch(Property::isSensitive);
   }
 
   private <T extends Annotation> boolean hasAnnotation(Class<T> annotationType) {
-    Logger log = LoggerFactory.getLogger(getClass());
-    try {
-      for (Annotation a : getClass().getField(name()).getAnnotations()) {
-        if (annotationType.isInstance(a)) {
-          return true;
-        }
-      }
-    } catch (SecurityException | NoSuchFieldException e) {
-      log.error("{}", e.getMessage(), e);
-    }
-    return false;
+    return getAnnotation(annotationType) != null;
   }
 
   private <T extends Annotation> T getAnnotation(Class<T> annotationType) {
-    Logger log = LoggerFactory.getLogger(getClass());
     try {
-      for (Annotation a : getClass().getField(name()).getAnnotations()) {
-        if (annotationType.isInstance(a)) {
-          @SuppressWarnings("unchecked")
-          T uncheckedA = (T) a;
-          return uncheckedA;
-        }
-      }
+      return getClass().getField(name()).getAnnotation(annotationType);
     } catch (SecurityException | NoSuchFieldException e) {
-      log.error("{}", e.getMessage(), e);
+      LoggerFactory.getLogger(getClass()).error("{}", e.getMessage(), e);

Review Comment:
   Maybe. I assumed that this was done this way previously because this is an enum, not a regular class. There may have been a bootstrapping issue with classloading because of the way enums are handled. I doubt this is a problem, but I did notice that one benefit here is that if there's no error (and there never should be for these), then there's no reason to bootstrap logging. This results in fewer spam messages about a logger sink not being configured in my IDE and in some tests.
   
   Since I didn't actually change this here... and merely inlined the variable, I'm inclined to leave it this way.



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