8154956: Module system implementation refresh (4/2016)

[Alan Bateman]

On 30/04/2016 15:58, Claes Redestad wrote:
>
> String fn is potentially calculated twice and could be moved to 
> directly after the first if (also gets rid of the odd looking "} if" 
> construct):
The "odd looking } if" shouldn't be there, it's benign but fat fingered 
in the editor I assume. For hidden or not-a-module files then the file 
name is created twice but not really an issue. We can make it a bit more 
readable though so good to point this out.


> :
>
> On line 448 in the same file a Set (serviceNames) is created from a 
> stream just to iterate over it and could be replaced by 
> distinct().forEach(...)
Some of the files in META-INF/services may not be service configuration 
files so this is why toServiceName returns an optional.


> :
>
> hashCode should always store a non-zero value to hash to avoid 
> recalculation in degenerate cases, e.g., if the calculated hash is 0, 
> store -1.
Okay, I think this pattern is itself too.


>
> http://cr.openjdk.java.net/~alanb/8154956/1/jdk/src/jdk.jartool/share/classes/sun/tools/jar/Main.java.udiff.html 
>
> http://cr.openjdk.java.net/~alanb/8154956/1/jdk/src/jdk.jlink/share/classes/jdk/tools/jmod/JmodTask.java.udiff.html 
>
>
> This adds quite a bit of code duplication between these tools - 
> transpose() etc - could (some of) this be refactored to a shared utility?
>
Yes, this is something that Mandy and I had a brief discussion recently 
and I think it's something for future cleanup.

-Alan