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?
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())
s'marks
Regarding the disparity between MatchResult and Matcher. I think that would
require a new sub-interface of MatchResult from which Matcher extends from and
returns. If we think it important we should do that for 9 otherwise we will be
stuck for the stream-based methods.
Why do you think we need a new sub-interface? Wouldn't it be sufficient to add
default methods?
What would the defaults do? Throwing an exception seems a poor solution to me.
My guess it will be possible to evolve Matcher in binary and source compatible
way to use the sub-type.
OK, I filed an RFE to cover this, then immediately closed it as a duplicate :-)
because I failed to search for the existing RFE that covers this:
https://bugs.openjdk.java.net/browse/JDK-8065554
Either we add some default methods that throw exceptions (gross) or we add a
sub-interface (also gross). It might be a matter of taste, or bike shed
painting.
Or a matter of how much the grossness is contained. In that respect the latter
is more self-contained, the latter spreads out to all independent consumers of
MatchResult.
Paul.
