[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[GitHub] brooklyn-server pull request #982: Improve coercions esp with generics


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()) {`.


---