That's a nice approach.

http://gwt-code-reviews.appspot.com/993801/diff/1/2
File user/src/com/google/gwt/uibinder/rebind/model/OwnerFieldClass.java
(right):

http://gwt-code-reviews.appspot.com/993801/diff/1/2#newcode46
user/src/com/google/gwt/uibinder/rebind/model/OwnerFieldClass.java:46:
private static HashMap<String, Integer> typeRank;
Make this final. You can build it in another map in your static
initializer, and fill in typeRank via Collections.unmodifiableMap()

http://gwt-code-reviews.appspot.com/993801/diff/1/2#newcode51
user/src/com/google/gwt/uibinder/rebind/model/OwnerFieldClass.java:51:
typeRank.put("java.lang.Boolean", 2);
Seems like you could rank the java.lang types as 3, so that you prefer
setBaz(boolean) to setBaz(Boolean). (Remember to add that test if you
make this change, and to set your default cost to 4. And you might want
to put that default cost in a constant next to this map.)

http://gwt-code-reviews.appspot.com/993801/diff/1/2#newcode186
user/src/com/google/gwt/uibinder/rebind/model/OwnerFieldClass.java:186:
!sameParameterTypes(preferredMethod, method)) {
The sameParameterTypes call is required to distinguish overrides from
super methods, right? Should mention that, and be sure unit test covers
it.

http://gwt-code-reviews.appspot.com/993801/diff/1/2#newcode334
user/src/com/google/gwt/uibinder/rebind/model/OwnerFieldClass.java:334:
* Ranks given method based on parameter conversion cost.
Please document the ranking here, like you did in the patch description

http://gwt-code-reviews.appspot.com/993801/diff/1/2#newcode362
user/src/com/google/gwt/uibinder/rebind/model/OwnerFieldClass.java:362:
private boolean sameParameterTypes(final JMethod m1, final JMethod m2) {
why?

http://gwt-code-reviews.appspot.com/993801/diff/1/3
File
user/test/com/google/gwt/uibinder/rebind/model/OwnerFieldClassTest.java
(right):

http://gwt-code-reviews.appspot.com/993801/diff/1/3#newcode146
user/test/com/google/gwt/uibinder/rebind/model/OwnerFieldClassTest.java:146:
public void testCheckBoxValueSetters() throws Exception {
Thanks for the test, but you need more to cover your various ranking
scenarios.

http://gwt-code-reviews.appspot.com/993801/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to