Regex Point Lambdafication Patch

Remi Forax forax at
Sun Mar 10 06:39:33 PDT 2013

On 03/10/2013 01:41 PM, Ben Evans wrote:
> Hi,
> I've completed a first cut of the point lambdafication for regexps patch.
> This is my first time running webrev - so please let me know if I've
> done this right - I'm supposed to host the webrev for the patch on a
> webserver somewhere?
> Assuming this is the case, I've put it up at:
> What should I do now?
> Thanks,
> Ben

Hi Ben,
I can be your sponsort (if anybody agree) and here is my review:

As a general comment, avoid abbreviation in the code,
CharSeqSpliterator should be CharSequenceSpliterator, cur should be current,
etc. A lot of people are not native english reader and abbreviation of word
can easily have another meaning in another latin language.
(BTW, what's the meaning of tmpCs ?)

asPredicate uses find() instead of matches which is weird given the doc 
also I think it shpuld return a Predicate<CharSequence> given that a Matcher
is created on a CharSequence.

for splitAsStream(), the javadoc comment talk about an array instead of 
a stream ?
Instead of See the split() method for more information, you can use @see.

The CharSeqSpliterator should be static if you move the creation
of the  Matcher from the constructor to the method splitAsStream, you 
only need
the Pattern to create the Matcher then you don't need it anymore so no need
to capture it.

Checking if the in sequence is null an replacing it by "" is very wrong 
in my opinion,
people should never expect a method to work if they send null as parameter.
Given that matcher(null) already throws a NPE, I think we should keep 
the same

If you combine the two points above, your constructor should be only 
that initialize values (this.x = x; this.y = y, ...) and not more, which 
is a good thing.

In CharSeqSpliterator.tryAdvance, instead of
   if (cur == input.length()) {
     return false;
   return true;
you can just write
   return cur != input.length;


More information about the lambda-dev mailing list