RFR: 8164781: Pattern.asPredicate specification is incomplete

Stuart Marks stuart.marks at oracle.com
Wed Apr 4 00:43:01 UTC 2018


Hi Vivek,

Thanks for taking on this task.

In case it wasn't clear from Paul's mail, what I think you should do is continue 
with this fix as a doc-only (and test-only) change, and not modify the behavior 
of this method. Doing that would be an incompatible change.

Uwe's point is a reasonable one, which is that you can't tell from the method 
name "asPredicate" whether it uses find() or match() semantics. Oh well, I think 
we just have to live with this, and document it clearly.

Adding a method to create a Predicate that has match() semantics would be a fine 
task to consider separately.

Also, in RegExTest.java,

4686         if (p.test("word1234")) {
4687             failCount++;
4688         }

I think the logic should be negated, as the predicate should properly find the 
pattern in this string.

Thanks,

s'marks

On 4/2/18 10:56 AM, Paul Sandoz wrote:
> Hi,
> 
> Looks good, expect for:
> 
> 5823      * @return  The predicate which can be used for finding on a string
> 
> “finding on a… ” is a little awkward to parse . I recommend to either change it back, since the first sentence of the method doc says what it means by matches, or being a little more verbose:
> 
>    The predicate which can be used for finding a match on a subsequence of a string
> 
> You will need a CSR to document the clarification in specification behavior.
> 
>> 
> To Uwe’s point, we could have chosen a more descriptive method name, e.g. asFinding/Predicate, leaving logical space for say any future asMatching/Predicate if we chose to add it.
> 
> Paul.
> 
> 
>> On Apr 1, 2018, at 1:11 AM, Vivek Theeyarath <vivek.theeyarath at oracle.com> wrote:
>>
>> Hi all,
>>
>>                Please review.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8164781
>>
>> Webrev: http://cr.openjdk.java.net/~rraghavan/8164781/webrev.01/
>>
>>
>>
>> Regards
>>
>> Vivek
> 


More information about the core-libs-dev mailing list