review request for 6798511/6860431: Include functionality of Surrogate in Character

Martin Buchholz martinrb at google.com
Sun Mar 21 07:14:44 UTC 2010


On Sat, Mar 20, 2010 at 17:30, Ulf Zibis <Ulf.Zibis at gmx.de> wrote:
> Fast path for BMP. +1
>
> I don't find ensureCapacityInternal() ???

My patches are dependent on each other.
I've changed my patch publishing script to
publish my entire .hg/patches/ subrepo, so that
others can import my patches, including their order.

http://cr.openjdk.java.net/~martin/webrevs/openjdk7/patches/

> shorter:
>    public AbstractStringBuilder appendCodePoint(int codePoint) {
>        int count = this.count;

You need to update this.count, not count, below.
To avoid this very common class of bugs,
I use final when I cache a field in a local of the same name.

Anyways, I adopted your caching of count, in
isBMPCodePoint2.

>        if (Character.isBMPCodePoint(codePoint)) {
>            ensureCapacityInternal(count + 1);
>            value[count++] = (char) codePoint;
>        } else if (Character.isValidCodePoint(codePoint)) {
>            ensureCapacityInternal(count + 2);
>            count += Character.toSurrogates(codePoint, value, count);

I'm sorry, I dislike methods that always return the same
value, just to make some client code a little shorter.
Character.toSurrogates should return void.

Martin



More information about the core-libs-dev mailing list