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
