RFR 8071479: Stream and lamdification improvements to j.u.regex.Matcher

[Paul Sandoz]

On Feb 13, 2015, at 1:20 AM, Stuart Marks <stuart.marks at oracle.com> wrote:

> 
> 
> On 2/12/15 3:15 AM, Paul Sandoz wrote:
>>   http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8071479--Matcher-stream-results/webrev/
> 
> OK, overall looks pretty good. Two minor comments on Matcher.java:
> 
> 1202       if (expectedCount >= 0 && expectedCount != matchOrResetCount)
> 1203           return true;
> 
> This is a concurrent modification check, so other cases that test this condition will throw CME. Should this also throw CME or should it indeed return true?
> 

The latter, so a CME is only thrown on data returning methods. 

I have added a comment:

// Defer throwing ConcurrentModificationException to when
// next is called.  The is consistent with other fail-fast
// implementations.
if (expectedCount >= 0 && expectedCount != matchOrResetCount)
    return true;

Given that the iterator is never exposed directly it most likely does not matter, but i wanted to be consistent.


> 1224                 // Perform a first find if required
> 1225                 if (s < 1 && !find())
> 1226                     return;
> 
> If I understand this correctly, the state field can have values -1, 0, or 1; and 's' is a local variable that's initialized from the state field.
> 
> I was confused by this code because it ends up calling find() when s == 0, which means "not found" ... so why is find() being called again in this case?
> 
> However, the s == 0 case is dispensed with earlier, so it actually cannot occur here. I think it would be clearer, and have the same behavior, if the condition were changed to
> 
>    if (s < 0 && !find())
> 

Ok.

Webrev updated in place:

http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8071479--Matcher-stream-results/webrev/src/java.base/share/classes/java/util/regex/Matcher.java.sdiff.html

Thanks,
Paul.