dlmarion commented on code in PR #2695:
URL: https://github.com/apache/accumulo/pull/2695#discussion_r872375137
##########
core/src/main/java/org/apache/accumulo/core/conf/Property.java:
##########
@@ -1828,24 +1785,24 @@ public static <T> T
createInstanceFromPropertyName(AccumuloConfiguration conf, P
// Precomputing information here avoids :
// * Computing it each time a method is called
// * Using synch to compute the first time a method is called
- for (Property p : Property.values()) {
- propertiesByKey.put(p.getKey(), p);
- if (p.getType().equals(PropertyType.PREFIX)) {
- validPrefixes.add(p.getKey());
- } else {
- validProperties.add(p.getKey());
- }
- // exclude prefix types (prevents setting a prefix type like
table.custom or
- // table.constraint, directly, since they aren't valid properties on
their own)
- if (!p.getType().equals(PropertyType.PREFIX)
- && p.getKey().startsWith(Property.TABLE_PREFIX.getKey())) {
- validTableProperties.add(p.getKey());
- }
- }
+ Predicate<Property> isPrefix = p -> p.getType() == PropertyType.PREFIX;
+ Arrays.stream(Property.values())
+ // record all properties by key
+ .peek(p -> propertiesByKey.put(p.getKey(), p))
+ // save all the prefix properties
+ .peek(p -> {
+ if (isPrefix.test(p))
+ validPrefixes.add(p.getKey());
+ })
+ // only use the keys for the non-prefix properties from here on
+ .filter(isPrefix.negate()).map(Property::getKey)
+ // everything left is a valid property
+ .peek(validProperties::add)
+ // but some are also valid table properties
+ .filter(k -> k.startsWith(Property.TABLE_PREFIX.getKey()))
+ .forEach(validTableProperties::add);
Review Comment:
I don't understand the impetus to change everything to use streams. When
looking for information about the performance of for-loops vs streams it seems
the consensus is that for-loops are faster unless you have a small set of data
to iterate over or you are using parallel streams.
##########
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:
We should create a static final Logger in the class.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
