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

Kumar Srinivasan kumar.x.srinivasan at oracle.com
Thu Nov 11 19:34:46 UTC 2010


Hi Alan, Brian,

Thanks! I will look into both your comments.

> Kumar - I just scanned this (sorry, I don't have time to do a detailed 
> review).
>
> - is Attribute.isEmpty right? or rather should ""==null be replaced by 
> false?
yes, I will redo this.

> - in BandStructure.java, can basicCodingIndexes be final?
it is final am I missing something ?

> - can Package.getHighestClassVersion use for-each? Same thing in 
> ClassWriter.writeAttributes.
oops I think I missed this one I think Brian also noticed this will fix 
them.


> - the indentation in PackerImpl.java at line 143
will do.

> - you've renamed locals in several places - is that because they are 
> hiding fields or something else?
right it is a NB nag saying the variable hides a field.
> - does the new switch statement in Driver.main need a default? (just 
> wondering if FindBugs will notice that).
No it did not, but I will recheck.

Kumar

>
> -Alan.




More information about the core-libs-dev mailing list