Review request 8002212 - adding read/writeObject to additional SerialXXX classes

Remi Forax forax at univ-mlv.fr
Fri Nov 2 23:42:51 UTC 2012


On 11/02/2012 11:57 PM, Lance Andersen - Oracle wrote:
> This is similar to 8001536, just additional classes.
>
> This adds read/writeObject, equals, clone methods to additional SerialXXX classes
>
> SQE, JCK and JDBC Unit tests all pass.
>
> The webrev can be viewed at http://cr.openjdk.java.net/~lancea/8002212/webrev.00

Hi Lance,
in SerialArray.equals(), I prefer

  return baseType == sa.baseType &&
         baseTypeName.equals(sa.baseTypeName)) &&
         Arrays.equals(elements, sa.elements);

instead of

   if(baseType == sa.baseType && baseTypeName.equals(sa.baseTypeName)) {
       return Arrays.equals(elements, sa.elements);
   }

In SerialDataLink, do you really need readObject/writeObject given
that you call the default implementations.

in SerialJavaObject, in equals, you should declare a local variable like 
in SerialDataLink.equals,
even if the local varialble is used once, it's more readable.
Also like in SerialArray.equals, you can do a return directly instead of 
if(...) return true.
in clone(), you can use the diamond syntax,
sjo.chain = new Vector<>(chain);
in setWarning(), you can use the diamond syntax as the original source does.
and in readObject, you can use the diamond syntax too.
In readObject, you forget to throw an exception if there are some static 
fields
in getClass().getFields() as the constructor does
(this code can be moved in a private static method).
Also, you should add a comment that because you call getClass() on obj,
there is an implicit null check.

This can be fixed as a separated bug or not but the code of
method SerialJavaObject.getField() is weird, the code checks if fields 
can be null,
but fields is never null. Also, cloning the field array is perhaps a 
better idea
if the reflection implementation doesn't cache the array of fields.

In SerialRef.equals, again, if(...) return should be transformed into 
return ...

>
> Best
> Lance

cheers,
Rémi

>
> Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
> Oracle Java Engineering
> 1 Network Drive
> Burlington, MA 01803
> Lance.Andersen at oracle.com
>




More information about the core-libs-dev mailing list