RFR: 8163798: Create a JarFile versionedStream method

Paul Sandoz paul.sandoz at oracle.com
Mon Sep 12 20:02:29 UTC 2016


> On 12 Sep 2016, at 12:36, Steve Drach <steve.drach at oracle.com> wrote:
> 
>>> I made a simple change, the new webrev is http://cr.openjdk.java.net/~sdrach/8163798/webrev.02/ <http://cr.openjdk.java.net/~sdrach/8163798/webrev.02/>
>>> 
>> 
>> I don’t like the state interplay between allowedVersions and getBaseSuffix, and the filtering for null. Consider merging filter.map.filter into a single flatMap.
> 
> Moved it into a map as Claes suggested
> 

Ok, better. Now there is no need to create an instance of VersionedStream, all methods can be static.

I wonder if you can combine the checks for the index:

  64             int index = name.indexOf('/', META_INF_VERSIONS_LEN);
  65             if (index == -1) {

  77             if (++index < name.length()) {

e.g. no slash, or found at end (not just malformed since we want to skip version directory entries if present?)

  if (index == -1 || index == name.length() - 1)


>> 
>> Also can getJarEntry ever return null?
> 
> yes, that’s why it’s filtered out
> 

Under what circumstances in this case can it return null? since you have iterated over the existing entries and reduced to a distinct subset.

Paul.

>> 
>> Claes makes a good point regarding performance. I would suggest getting this functional and tested then tweaking for performance.
>> 
>> Paul.



More information about the core-libs-dev mailing list