http://codereview.appspot.com/14088/diff/2001/3006 File java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanJsonConverter.java (right):
http://codereview.appspot.com/14088/diff/2001/3006#newcode77 Line 77: // Ensure consistent method ordering by using a linked hash map. On 2009/03/17 15:09:44, awiner wrote:
comment doesn't match code (didn't before, either)
Done. http://codereview.appspot.com/14088/diff/2001/3006#newcode94 Line 94: String name = method.getName(); On 2009/03/17 15:09:44, awiner wrote:
this includes static methods as well as non-static; and methods with
multiple
args or no args. Shouldn't it be a bit more restrictive (at least for
the
no-annotation codepath)?
Possibly, though I didn't think it would make much of a difference one way or another, and the code is more complex to support the extra level of filtering. http://codereview.appspot.com/14088/diff/2001/3006#newcode139 Line 139: return value; On 2009/03/17 15:09:44, awiner wrote:
return Boolean.TRUE.equals(value)? Or
"true".equals(String.valueOf(type)); Done. http://codereview.appspot.com/14088/diff/2001/3006#newcode175 Line 175: } else if (Collection.class.isAssignableFrom(clazz)) { On 2009/03/17 15:09:44, awiner wrote:
List<T> is missing
How so? Collection.class.isAssignableFrom should cover any collection type, including lists. Set is explicit because it's meaningfully different when requested, but if it wasn't requested explicitly, a List is the most accurate fall back type. http://codereview.appspot.com/14088/diff/2001/3006#newcode216 Line 216: for (Object o : type.getEnumConstants()) { On 2009/03/17 15:09:44, awiner wrote:
could be just Enum.valueOf(type, value);
This doesn't work for the enums that have overridden toString though, which happens in the Java -> JSON mappings. http://codereview.appspot.com/14088/diff/2001/3006#newcode240 Line 240: Object out = injector.getInstance(Key.get(type)); On 2009/03/17 15:09:44, awiner wrote:
injector.getInstance(type);
getInstance doesn't work on Type, it works on Class<?>, or Key. I'd have to cast to Class<?>, which would make parameterized type population break. http://codereview.appspot.com/14088
