Please extract the expando work to the gwt.core.client package and add test cases for it, as it's a generally-useful facility to offer.
Needs tests of the JDO testcases. @Cromwellian, The expando field just reserves a bit of the Object-field namespace since it's lazily initialized. http://gwt-code-reviews.appspot.com/47807/diff/1/9 File user/src/com/google/gwt/user/client/rpc/WeakMapping.java (right): http://gwt-code-reviews.appspot.com/47807/diff/1/9#newcode16 Line 16: package com.google.gwt.user.client.rpc; Move this to the core.client package, since it's not specific to the rpc system. http://gwt-code-reviews.appspot.com/47807/diff/1/9#newcode22 Line 22: * A class associating a (key -> value) map with arbitrary sourceobjects. A WeakHashMap source objects http://gwt-code-reviews.appspot.com/47807/diff/1/9#newcode24 Line 24: * garbage collected, the map associated with it will be collected as well. This javadoc should accurately describe both hosted and web-mode code. The specifics of the implementation are usually documented as a regular block comment just below the class declaration. http://gwt-code-reviews.appspot.com/47807/diff/1/9#newcode31 Line 31: public static Object get(Object instance, String key) { Needs documentation, especially on the namespace of the key. http://gwt-code-reviews.appspot.com/47807/diff/1/8 File user/src/com/google/gwt/user/rebind/rpc/FieldSerializerCreator.java (right): http://gwt-code-reviews.appspot.com/47807/diff/1/8#newcode159 Line 159: Class annotationClass = Class.forName("javax.jdo.annotations.PersistenceCapable"); Raw type http://gwt-code-reviews.appspot.com/47807/diff/1/5 File user/src/com/google/gwt/user/server/rpc/impl/Base64Utils.java (right): http://gwt-code-reviews.appspot.com/47807/diff/1/5#newcode2 Line 2: * Copyright 2008 Google Inc. Copyright date should be current for new files. http://gwt-code-reviews.appspot.com/47807/diff/1/5#newcode19 Line 19: * A utility to decode and encode byte arrays as Strings, using only "safe" characters. Formatting looks off. http://gwt-code-reviews.appspot.com/47807/diff/1/7 File user/src/com/google/gwt/user/server/rpc/impl/SerializabilityUtil.java (right): http://gwt-code-reviews.appspot.com/47807/diff/1/7#newcode315 Line 315: * Returns true if the given field is named 'jdoDetachedState' and the enclosing Formatting http://gwt-code-reviews.appspot.com/47807/diff/1/7#newcode322 Line 322: Class[] interfaces = field.getDeclaringClass().getInterfaces(); If A extends B; B implements I; does A.class.getInterfaces() include I? Would it be easier to have a static field that's initialized with a Class.forName and Class.isInstance() used instead? http://gwt-code-reviews.appspot.com/47807/diff/1/4 File user/src/com/google/gwt/user/server/rpc/impl/ServerSerializationStreamReader.java (right): http://gwt-code-reviews.appspot.com/47807/diff/1/4#newcode611 Line 611: // Check for the presence of the javax.jdo.spi.Detachable interface. If it is present, Use a block comment so the formatter will format this correctly. http://gwt-code-reviews.appspot.com/47807/diff/1/4#newcode614 Line 614: Class[] interfaces = instanceClass.getInterfaces(); Would a speculatively-initialized Class literal field and Class.isInstance() work more correctly? http://gwt-code-reviews.appspot.com/47807/diff/1/4#newcode676 Line 676: assert version == 1; This should throw a runtime exception, since end-users likely won't have assertions turned on. http://gwt-code-reviews.appspot.com/47807/diff/1/4#newcode682 Line 682: case 'N': What do these values mean? http://gwt-code-reviews.appspot.com/47807/diff/1/4#newcode689 Line 689: Class c = Class.forName(className); Use of raw types should be flagged by eclipse. Class<? extends Externalizable> c = Class.forName(className).asSubclass(Externalizable.class) Add a ClassCastException to the catch block below. http://gwt-code-reviews.appspot.com/47807/diff/1/6 File user/src/com/google/gwt/user/server/rpc/impl/ServerSerializationStreamWriter.java (right): http://gwt-code-reviews.appspot.com/47807/diff/1/6#newcode666 Line 666: // Check for the presence of the javax.jdo.spi.Detachable interface. If it is present, Block comment and formatting. http://gwt-code-reviews.appspot.com/47807/diff/1/6#newcode669 Line 669: Class[] interfaces = instanceClass.getInterfaces(); Class.isInstance http://gwt-code-reviews.appspot.com/47807/diff/1/6#newcode713 Line 713: * null is indicated by 'N', an externalizable field is indicated by an 'E' Use static final fields for these values, and reference them with the @value javadoc tag. http://gwt-code-reviews.appspot.com/47807/diff/1/6#newcode734 Line 734: out.writeInt(1); Extract the number 1 to a constant. http://gwt-code-reviews.appspot.com/47807/diff/1/10 File user/super/com/google/gwt/emul/java/lang/Object.java (right): http://gwt-code-reviews.appspot.com/47807/diff/1/10#newcode45 Line 45: * an expando containing a String -> Object mapping on arbitrry Objects. arbitrary http://gwt-code-reviews.appspot.com/47807/diff/1/10#newcode50 Line 50: private transient Object expando; Sort order isn't enforced by checkstyle, but fields should generally be sorted. This field should be declared as JavaScriptObject type. http://gwt-code-reviews.appspot.com/47807/diff/1/11 File user/super/com/google/gwt/user/translatable/com/google/gwt/user/client/rpc/WeakMapping.java (right): http://gwt-code-reviews.appspot.com/47807/diff/1/11#newcode21 Line 21: public class WeakMapping { Needs javadoc. The eclipse checkstyle rules should have put a warning here. http://gwt-code-reviews.appspot.com/47807/diff/1/11#newcode27 Line 27: [email protected]::expando[key] = value; You need to prefix the key in order to prevent conflicts with builtin properties; typicially expando[':' + key] http://gwt-code-reviews.appspot.com/47807/diff/1/3 File user/test/com/google/gwt/user/server/rpc/RPCTest.java (right): http://gwt-code-reviews.appspot.com/47807/diff/1/3#newcode167 Line 167: public static void testBase64Utils() { Move these to their own test class. http://gwt-code-reviews.appspot.com/47807 --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
