http://gwt-code-reviews.appspot.com/1420808/diff/1/user/src/com/google/gwt/uibinder/rebind/TypeOracleUtils.java File user/src/com/google/gwt/uibinder/rebind/TypeOracleUtils.java (right):
http://gwt-code-reviews.appspot.com/1420808/diff/1/user/src/com/google/gwt/uibinder/rebind/TypeOracleUtils.java#newcode60 user/src/com/google/gwt/uibinder/rebind/TypeOracleUtils.java:60: JType... argTypes) { On 2011/04/22 01:13:17, rjrjr wrote:
I don't know that I would ever use this unless it was based on JClassType.getOverridableMethods().
But rather than hardcoding that choice, perhaps you should make the
first
argument a JMethod[] to let me choose the set myself? Can put @see
pointing to
getOverridableMethods.
Ok, how about a method taking the method list and one taking the type, which defaults to either getMethods or getOverridableMethods (whichever you think is more useful). I would think in this application it is most interesting in this context, since you want to know if you can call the method with a set of parameters, and whether it is implemented on that class or a superclass is just an implementation detail.
I'm biting my tongue about the fact that this method is unused. I'm
generally
opposed to speculative coding, but I can imagine cleaning up some
existing code
with this.
It just seemed an obvious extension of the constructor code, but it is certainly easy enough to leave it out if you prefer. http://gwt-code-reviews.appspot.com/1420808/diff/1/user/src/com/google/gwt/uibinder/rebind/TypeOracleUtils.java#newcode65 user/src/com/google/gwt/uibinder/rebind/TypeOracleUtils.java:65: if (method.isStatic() == isStatic && methodName.equals(method.getName()) On 2011/04/22 01:13:17, rjrjr wrote:
could you put () around the == statement?
Done. http://gwt-code-reviews.appspot.com/1420808/diff/1/user/src/com/google/gwt/uibinder/rebind/TypeOracleUtils.java#newcode78 user/src/com/google/gwt/uibinder/rebind/TypeOracleUtils.java:78: * @param argType On 2011/04/22 01:13:17, rjrjr wrote:
What's the difference between a paramType and an argType? Seems like
the
distinction matters here, but param and arg look like synonyms to me.
paramType is the type of the formal parameter declaration, while argType is the type of the argument to be passed in.
Should the args be leftHandType and rightHandType? This applies
throughout the
class of course, not just here.
That would work, although the terminology seems a bit odd in the context of a method call. http://gwt-code-reviews.appspot.com/1420808/diff/1/user/src/com/google/gwt/uibinder/rebind/TypeOracleUtils.java#newcode140 user/src/com/google/gwt/uibinder/rebind/TypeOracleUtils.java:140: JType[] ctorArgTypes = paramTypes; On 2011/04/22 01:13:17, rjrjr wrote:
Looks like ctorArgTypes is refactoring chaff, should be inlined.
Yes, I created it just so I could inline it, then never did :). Done. http://gwt-code-reviews.appspot.com/1420808/diff/1/user/src/com/google/gwt/uibinder/rebind/TypeOracleUtils.java#newcode157 user/src/com/google/gwt/uibinder/rebind/TypeOracleUtils.java:157: } else { On 2011/04/22 01:13:17, rjrjr wrote:
else if
Done. http://gwt-code-reviews.appspot.com/1420808/diff/1/user/test/com/google/gwt/uibinder/rebind/TypeOracleUtilsTest.java File user/test/com/google/gwt/uibinder/rebind/TypeOracleUtilsTest.java (right): http://gwt-code-reviews.appspot.com/1420808/diff/1/user/test/com/google/gwt/uibinder/rebind/TypeOracleUtilsTest.java#newcode29 user/test/com/google/gwt/uibinder/rebind/TypeOracleUtilsTest.java:29: On 2011/04/22 01:13:17, rjrjr wrote:
import static com.google.gwt.uibinder.rebind.TypeOracleUtils.*; might
make this
easier to read.
Done. http://gwt-code-reviews.appspot.com/1420808/diff/1/user/test/com/google/gwt/uibinder/rebind/TypeOracleUtilsTest.java#newcode145 user/test/com/google/gwt/uibinder/rebind/TypeOracleUtilsTest.java:145: public void testHasCompatibleMethod() { On 2011/04/22 01:13:17, rjrjr wrote:
Many lines too long. Did you auto format?
Yes, although I did make some edits since then. The @link tags can't be split, and Reitveld is wrapping at 80 instead of 100. I will make sure it is formatted properly before submitting. http://gwt-code-reviews.appspot.com/1420808/diff/1/user/test/com/google/gwt/uibinder/rebind/TypeOracleUtilsTest.java#newcode146 user/test/com/google/gwt/uibinder/rebind/TypeOracleUtilsTest.java:146: assertTrue("didn't find static m1(Child)", TypeOracleUtils.hasCompatibleMethod(foo, true, "m1", On 2011/04/22 01:13:17, rjrjr wrote:
s/didn't find/Should have found/
Done. http://gwt-code-reviews.appspot.com/1420808/diff/1/user/test/com/google/gwt/uibinder/rebind/TypeOracleUtilsTest.java#newcode148 user/test/com/google/gwt/uibinder/rebind/TypeOracleUtilsTest.java:148: assertFalse("found m1(Child)", TypeOracleUtils.hasCompatibleMethod(foo, false, "m1", child)); On 2011/04/22 01:13:17, rjrjr wrote:
s/found/Should not have found/
Done. http://gwt-code-reviews.appspot.com/1420808/diff/1/user/test/com/google/gwt/uibinder/rebind/TypeOracleUtilsTest.java#newcode215 user/test/com/google/gwt/uibinder/rebind/TypeOracleUtilsTest.java:215: protected void setUp() throws Exception { On 2011/04/22 01:13:17, rjrjr wrote:
please throw specific exception types
This was just left by the automatic stub generation in Eclipse -- I'll tighten it down. http://gwt-code-reviews.appspot.com/1420808/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
