Mike,
Here is the new revision:
http://cr.openjdk.java.net/~ksrini/6990106/webrev.03/
I nailed all the items per your suggestions.
A few more I nits I noticed.
BandStructure : allKQBands could be final.
ClassReader : string switch opportunities in readAttributes()
FixedArrayList : Seems to be unchanged since last webrev. Was nothing saved by
extending AbstractList?
Initially I was extending AbstractList and later changed it to
implement List.
And no I did not see any change.
Thanks
Kumar
ConstantPool : equals() only works as long as all sub-classes have different
tag values. (see sameTagAs() which uses instanceof rather than getClass()).
Using getClass() likely makes tag comparison unnecessary.
Package : another InnerClass.equals() using instanceof
PackerImpl : several uses of XXX.size()> 0 which could be !XXX.isEmpty() which
is generally preferred as size() is not O(1) for some collections where isEmpty()
is O(1).
PropMap : could _listeners and theMap be private? Since initialization of
theMap doesn't depend upon any constructor params why not construct it at
declaration?
Mike
On Nov 18 2010, at 14:58 , Kumar Srinivasan wrote:
Hi Mike,
Thanks for the review, here is the new version:
http://cr.openjdk.java.net/~ksrini/6990106/webrev.02/
