thanks for feedback, submitting
http://codereview.appspot.com/17086/diff/1/2 File java/common/src/main/java/org/apache/shindig/protocol/DefaultHandlerRegistry.java (right): http://codereview.appspot.com/17086/diff/1/2#newcode379 Line 379: private static class MethodCaller { On 2009/02/19 23:15:35, awiner wrote:
move this to a package-private top-level class, maybe:
DefaultHandlerRegistry is
quite long as is
Think this will make sense as part of moving registry classes to a separate package. This class is definitely too big, but probably makes sense to split it all up at once. http://codereview.appspot.com/17086/diff/1/2#newcode384 Line 384: private final Constructor<?> requestItemConstructor; On 2009/02/19 23:15:35, awiner wrote:
should be Constructor<? extends RequestItem>
I couldn't find a way to do this without having to suppress warnings somewhere. http://codereview.appspot.com/17086/diff/1/2#newcode395 Line 395: public MethodCaller(Method method, boolean isRest) throws NoSuchMethodException { On 2009/02/19 23:25:45, louiscryan wrote:
On 2009/02/19 23:15:35, awiner wrote: > instead of isRest, use an inner enum; or just pass in > requestItemFirstParamClass; or just have some statics with the type
signature
> of the REST and RPC item constructors, and pass in the whole type
signature
> array to the constructor
or add getRestRequestItem and a getRpcRequestItem
Went with the two getter flavor. http://codereview.appspot.com/17086/diff/1/3 File java/common/src/test/java/org/apache/shindig/protocol/TestHandler.java (right): http://codereview.appspot.com/17086/diff/1/3#newcode103 Line 103: } On 2009/02/19 23:25:45, louiscryan wrote:
add test case for no-param method call
added http://codereview.appspot.com/17086
