3
\$\begingroup\$

I have a Java dynamic proxy with an invoke method that looks like this:

 @Override
public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
    Object result;
    try {
        //Not sure if this line is ok?
        Token token = (Token) args[0];

        System.out.println("Token with id: "+token.getId()+ " was passed");
        result = method.invoke(realObject, args);
    } catch (InvocationTargetException e) {
        throw e.getTargetException();
    } catch (Exception e) {
        throw new RuntimeException("unexpected invocation exception: " + e.getMessage());
    } finally {
        System.out.println("after method " + method.getName());
    }
    return result;
}

Would it be considered bad practice to pull out and cast that Token object from the args array?

The problem I can see is that it means the proxy isn't generic and in fact will only work for method invocations that pass an instance of a Token object as the first parameter.

\$\endgroup\$
  • 2
    \$\begingroup\$ Why the hassle to have a single return, while you have those throws in between? \$\endgroup\$ – abuzittin gillifirca Feb 13 '14 at 12:34
3
\$\begingroup\$

Casting is never 'pretty'.... But, in a proxy situation like this (assuming you really need the proxy), it is possibly unavoidable.

I can recommend though that you do it defensively... you should really do:

if (args.length > 0) {
    if (args[0] instanceof Token) {
        System.out.println("Token with id: "+((Token)args[0]).getId()+ " was passed");
    } else {
        System.out.println("Proxy for a method which does not have an initial token: " + args[0]);
    }
} else {
    System.out.println("Proxy for a method that is being passed no arguments.");
}

If your situation requires that these values are populated, then you should throw exceptions instead of the other println methods.

Also, you should consider checking that the number of arguments in the args matches the number of arguments expected in method.getParameterTypes(), but with var-args, that can be hard.

Additionally, this line:

throw new RuntimeException("unexpected invocation exception: " + e.getMessage());

Should incorporate the actual cause.... when you get an exception through this catch block, it will be one of those times when the cause is critical, and you are throwing the cause away! That exception should look like:

throw new RuntimeException("unexpected invocation exception: " + e.getMessage(), e);
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.