> On 12 Sep 2016, at 12:36, Steve Drach <steve.dr...@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.
