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

[Stuart Marks]

On 2/27/15 12:40 PM, Xueming Shen wrote:
> On 02/27/2015 11:21 AM, Xueming Shen wrote:
>> On 02/27/2015 10:55 AM, Paul Sandoz wrote:
>>>
>>> What about a light wright immutable MatchResult? is that possible?
>>
>> Should be possible. I can give it try.
>
> too repetitive?
>
> http://cr.openjdk.java.net/~sherman/regex.stream/src/java.base/share/classes/java/util/regex/Matcher.java.sdiff.html

Not too bad. I know I was surprised to see that Matcher itself is the only 
MatchResult implementation, and that there wasn't a lightweight immutable 
MatchResult separate from Matcher. So I'm glad to see this added.

(One thing to think about, not part of this particular change, is whether named 
matching groups could be added to or extend MatchResult. Doing that would change 
the implementation of the lightweight MatchResult. See JDK-8065554.)

A few comments on the implementation.

At line 302, end(group) calls the groupCount() method instead of returning the 
captured groupCount local.

The MatchResult anonymous class unavoidably captures a reference to 'this', 
which is the enclosing Matcher instance. I don't think it needs that, but it 
does enable methods like groupCount() on the enclosing class to be called 
inadvertently.

For these reasons the lightweight MatchResult might better be refactored to be a 
(named) static nested class (or even a top-level class). You'll have to write a 
constructor and declare fields instead of capturing locals though, but I think 
this makes it a bit more clear as to what's actually going on. For example, you 
could make all the fields final to make it clear that the object really is 
immutable.

s'marks