Regex Point Lambdafication Patch
Remi Forax
forax at univ-mlv.fr
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:
>
> http://www.java7developer.com/webrev-kittylyst.001/
>
> 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
comment
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
semantics.
If you combine the two points above, your constructor should be only
something
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;
cheers,
Rémi
More information about the lambda-dev
mailing list