brusdev commented on a change in pull request #3851:
URL: https://github.com/apache/activemq-artemis/pull/3851#discussion_r749325824
##########
File path:
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/DefaultSensitiveStringCodec.java
##########
@@ -79,6 +79,21 @@ public void init(Map<String, String> params) throws
Exception {
}
}
+ @Override
+ public boolean compare(Object value, Object encodedValue) {
+ if (encodedValue instanceof String == false) {
+ throw new IllegalArgumentException("Not supported encodedValue type:
" + encodedValue.getClass().getName());
+ }
Review comment:
> doing '== boolean' in an if is pretty ugly. Seems like either using
!(encodedValue instanceof String), or changing the if to wrap the overall
comparison and throw at the end would be nicer.
Nice suggestion moving it at the end
> This could also NPE generating the exception, on encodedValue.getClass().
I would not add another condition to check if value is null just to throw NPE
> "Unsupported" would seem a better fit rather than "Not supported".
I'll replace Not supported with Unsupported
--
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]
