Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/982#discussion_r214612997
--- Diff:
core/src/main/java/org/apache/brooklyn/util/core/flags/MethodCoercions.java ---
@@ -87,13 +89,23 @@ public boolean apply(@Nullable Method input) {
Method accessibleMethod =
Reflections.findAccessibleMethod(method).get();
try {
Type paramType = method.getGenericParameterTypes()[0];
- Object coercedArgument = TypeCoercions.coerce(argument,
TypeToken.of(paramType));
- return Maybe.of(accessibleMethod.invoke(instance,
coercedArgument));
+ Maybe<?> coercedArgumentM =
TypeCoercions.tryCoerce(argument, TypeToken.of(paramType));
+ RuntimeException exception =
Maybe.getException(coercedArgumentM);
--- End diff --
Looks wrong - this will cast `coercedArgumentM` to `Maybe.absent`, but on
the line below you do `coercedArgumentM.isPresent`. So presumably the call to
`getException` will sometimes throw a `ClassCastException`?
Ah, I see you've changed the semantics of `getException` to return null if
it wasn't an exception.
Why change it? I liked the way that asking for the exception when `present`
was a programming error - there will never be an exception when present. It
looks like you can easily avoid that by removing this line, and changing the if
statement below to just `if (coercedArgumentM.isAbsent()) {`.
---
