Please review java.util.jar.pack.* cleanup/refactoring/generificaiton

Kumar Srinivasan kumar.x.srinivasan at oracle.com
Fri Nov 19 19:10:35 UTC 2010


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/
>>




More information about the core-libs-dev mailing list