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

[Stuart Marks]

OK, this looks great. Thanks for the updates.

There is also

     "in same order" -> "in the same order"

in the doc for the results() method, as Brian pointed out internally.

No need for another webrev.

s'marks

On 2/13/15 1:17 AM, Paul Sandoz wrote:
>
> 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.
>