On 21/11/2011 5:39 AM, Rémi Forax wrote:
On 11/20/2011 08:14 PM, Alan Bateman wrote:
On 18/11/2011 18:27, Darryl Mocek wrote:
Hello. Please review this patch to fix a serialization issue with
String's CASE_INSENSITIVE_ORDER. If you serialize, then deserialize
the class, the equals test will fail in the comparison of what was
serialized with what was deserialized. Webrev, including test, can be
found here: http://cr.openjdk.java.net/~sherman/darryl/5035850/webrev

Thanks,
Darryl
This looks okay to me but I would suggest adding a comment to
readResolve, maybe something like "Replaces the de-serialized object"
as the causal reader may not know what this method is for.

-Alan.

Hi Darryl, Hi Alan,
additional comments: in the test, you don't need to initialize result to
null because you can remove the catch(Exception) block

Related to this I was going to say that if you get an unexpected exception I believe you should simply let it propagate to indicate failure.

and also you should use == instead of equals for the last check.

Given equals() is not overridden the two are equivalent. But testing both as Mike suggests would catch any erroneous redefinition of equals() in the future.

David
-----

cheers,
Rémi


Reply via email to