joewitt commented on code in PR #9600:
URL: https://github.com/apache/nifi/pull/9600#discussion_r1898626443


##########
nifi-extension-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/AbstractAMQPProcessor.java:
##########
@@ -139,23 +139,16 @@ abstract class AbstractAMQPProcessor<T extends 
AMQPWorker> extends AbstractProce
             .addValidator(StandardValidators.BOOLEAN_VALIDATOR)
             .build();
 
-    private static final List<PropertyDescriptor> propertyDescriptors;
-
-    static {
-        propertyDescriptors = List.of(
-                BROKERS,
-                HOST, PORT,
-                V_HOST,
-                USER,
-                PASSWORD,
-                AMQP_VERSION,
-                SSL_CONTEXT_SERVICE,
-                USE_CERT_AUTHENTICATION);
-    }
-
-    protected static List<PropertyDescriptor> getCommonPropertyDescriptors() {
-        return propertyDescriptors;
-    }
+    protected static final List<PropertyDescriptor> PARENT_PROPERTIES = 
List.of(
+            BROKERS,
+            HOST, PORT,
+            V_HOST,
+            USER,
+            PASSWORD,
+            AMQP_VERSION,
+            SSL_CONTEXT_SERVICE,
+            USE_CERT_AUTHENTICATION
+    );

Review Comment:
   Here the parent class 'getCommonPropertyDescriptors' method is removed and 
instead a subclass is supposed to know to use the static list of descriptors as 
they build their own lists.  I think this weakens what the author intended in 
building that class.
   
   Also there is at least one example much later on in this PR where you kept 
the parent method.  I think keeping the parent method better conveys intent to 
any subclassers.



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

Reply via email to