http://gwt-code-reviews.appspot.com/1601805/diff/1/user/src/com/google/web/bindery/autobean/shared/ValueCodex.java File user/src/com/google/web/bindery/autobean/shared/ValueCodex.java (right):
http://gwt-code-reviews.appspot.com/1601805/diff/1/user/src/com/google/web/bindery/autobean/shared/ValueCodex.java#newcode106 user/src/com/google/web/bindery/autobean/shared/ValueCodex.java:106: if (value.isNumber()) { This seems fine, but could you add a test to AutoBeanCodexTest to make sure it continues to work? http://gwt-code-reviews.appspot.com/1601805/diff/1/user/src/com/google/web/bindery/autobean/shared/ValueCodex.java#newcode114 user/src/com/google/web/bindery/autobean/shared/ValueCodex.java:114: long timestamp = ((Date) value).getTime(); As I said in the bug, I'm not sure this is a good idea. http://gwt-code-reviews.appspot.com/1601805/diff/1/user/src/com/google/web/bindery/autobean/shared/ValueCodex.java#newcode368 user/src/com/google/web/bindery/autobean/shared/ValueCodex.java:368: if (clazz.isEnum()) { On 2011/12/01 17:18:36, tbroyer wrote:
Oh, I don't know why I had that in this CL, but the rationale is: in
the only
case where findType can be called with an
"enum-value-with-overridden-methods'
class" (a subclass of the isEnum class), it's OK for findType to
return null
because the value will be "upcasted" (and ENUM.canUpcast will then
return true,
so it'll correctly be used); see encode(Object) above. So the question is whether to add an overhead for every encoded value
(checking
getSuperClass.isEnum here) or only for those specific enum values
(loop over the
types and ask them if they canUpcast the value). I'd rather say the
latter,
hence this change.
It looks like you're right that the answer will still be correct, but I'm not sure about the performance implications, so I'd be inclined to leave it as is unless you've done some performance testing showing that it's a win. http://gwt-code-reviews.appspot.com/1601805/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
