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

[Brian Goetz]

Mostly style issues, but at least one likely bug.

In a few places, you've used @SuppressWarnings("unchecked").  While this may 
well be unavoidable, it is worth factoring this out into the smallest possible 
chunk.  For BandStructure, you've applied it to the whole class, and there's 
some big methods that use it too.  You can usually get it down to a small 
"makeArrayOfGenericFoo" method, since this is the most common source of 
non-fixable unchecked warnings.

Also using an interface like Constants to import symbols is an antipattern and 
is better replaced with static imports.

In ClassReader you've replaced use of new Integer() and friends with 
Integer.valueOf(), but its better style to go all the way and just use 
autoboxing.

In Instruction the equals() method takes into account bc, w, length, and 
bytes, but the hashCode() method also takes into account pc.  This may violate 
the "equal objects must have equal hashcodes" rule.

Throughout you've changed loops from
   for (Iterator i=...)
to
   for (Iterator<T> i=...)
but didn't go all the way and just use foreach loops.

PropMap should extend TreeMap by composition, not extension.  This 
implementation is subject to the hazards outlined in the Effective Java item 
"Prefer composition to extension."  (For example you override put() but not 
putAll(), but this idiom cannot be made to work without tightly coupling it to 
the superclass implementation.)

On 11/11/2010 12:29 PM, Kumar Srinivasan wrote:
> Hi,
>
> Appreciate a review of the pack200 cleanup work, in the following areas:
>
> 1. General generification of Collections.
> 2. fixed worthy findbugs items.
> 3. fixed worthy Netbeans nags.
> 4. Elimination of array/generics combination.
>
> The webrev is here:
> http://cr.openjdk.java.net/~ksrini/6990106/webrev.00/
>
> Thanks
> Kumar
>