On Nov 3, 2012, at 11:14 AM, Remi Forax wrote: > On 11/03/2012 01:46 AM, Lance Andersen - Oracle wrote: >> Hi Remi, >> > [...] > >>> In SerialDataLink, do you really need readObject/writeObject given >>> that you call the default implementations. >> I thought about that but had decided to add them for consistency > > The serialization implementation needs to create metadata associated with > readObject/writeObject, > so if we can avoid to implement them, we reduce the runtime memory footprint > a little. > I would prefer to have a comment saying that default serialization is Ok here.
OK, I will just comment the methods out and add a comment as you suggest > >>> 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. >> OK >>> 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); >> Yeah, long story, but I forgot to reset to diamond syntax (will tell you >> over a beer sometime :-) ) > > sure, now or you have to visit Paris or I have to go to NY :) Or Boston, where I am based or perhaps a JavaOne ;-) > >>> in setWarning(), you can use the diamond syntax as the original source does. >>> and in readObject, you can use the diamond syntax too. >> OK >>> 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). >> I thought about that, but figured since it was already through that check >> when the object was created, I did not think it required repeating in the >> readObject method > > A serialization stream can be forged to encode a SerialJavaObject that > references an object that have a static field without creating a > SerialJavaObject in memory. True and there are tests in the test suite that basically do that. Anyways, I added that check in the revised webrev that I pushed earlier. > >>> Also, you should add a comment that because you call getClass() on obj, >>> there is an implicit null check. >> Would it be better to put an null check in explicitly? > > As you prefer :) > > [...] > >> Thank you again, will send an update webrev over the weekend >> >> Best >> Lance > > 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
