Here is revised webrev taking into account Remi's suggestions http://cr.openjdk.java.net/~lancea/8001536/webrev.01/
Best, Lance 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
