On 10/31/2012 04:08 PM, Lance Andersen - Oracle wrote:
Here is revised webrev taking into account Remi's suggestions 
http://cr.openjdk.java.net/~lancea/8001536/webrev.01/

looks good :)

Best,
Lance

Rémi


On Oct 30, 2012, at 2:05 PM, Remi Forax wrote:

On 10/30/2012 05:25 PM, Lance Andersen - Oracle wrote:
Hi,

This is a request for review of 
http://cr.openjdk.java.net/~lancea/8001536/webrev.00/.  This adds 
read/writeObject as well as clone methods to SerialXLob classes.

All SQE tests passed,  1 failure in the RowSet JCK/TCK tests due to a bug in 
the test that the TCK team is aware of and will address.  JDBC Unit tests all 
pass .
Hi Lance.
In SerialBlob and in SerialClob
test (obj == null) is not necessary in equals, null instanceof X is always 
false.

in hashCode, Objects.hash() allocate an array to pass arguments to 
Arrays.hashCode() and box primitive values to Object.
while this method is really convenient to use, each calls will allocate an 
array and box the two values,
the overhead seems to high here.
This code should be equivalent:
    return ((31 +Arrays.hashCode(buf)) * 31 +len) * 31 + origLen;

in clone, sb should not be initialized to null and the catch should be: throw 
new InternalError(e),
this is the standard code you can see in clone.

in readObject, the test (buf.length != len) can be done before decoding the 
blob.

in writeObject, you set "blob" twice, which is weird, also I think that if blob 
is not Serializable,
the code should throw an exception, so you should not use instanceof and let 
s.writeFields()
to throw NotSerializable exception.

cheers,
Rémi


Best
Lance

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



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


Reply via email to