Re: Add getChars to CharSequence

[Martin Buchholz]

Hi Ulf!


On Mon, Apr 22, 2013 at 9:11 PM, Ulf Zibis <ulf.zi...@cosoco.de> wrote:

> Am 22.04.2013 21:45, schrieb Martin Buchholz:
>
>> Another preliminary webrev is out at
>> http://cr.openjdk.java.net/~**martin/webrevs/openjdk8/**getChars/<http://cr.openjdk.java.net/~martin/webrevs/openjdk8/getChars/><
>> http://cr.openjdk.java.net/%**7Emartin/webrevs/openjdk8/**getChars/<http://cr.openjdk.java.net/%7Emartin/webrevs/openjdk8/getChars/>
>> >
>>
>> This looks great, but AbstractStringBuilder could be again simplified ...
>
> 1. What is the difference between replaceOutOfBounds() and
> substringOutOfBounds() ? ;-)
>
>
detail message is different to try to match the parameter names (most of
the time...)


> 2. naming suggestion:
>     outOfBoundsMsg() --> outOfSrcBoundsMsg()
>     replaceOutOfBounds(), substringOutOfBounds() --> outOfDstBoundsMsg()
>
> 3. You could rename the parameters of getChars to:
>     getChars(int start, int end, char[] dst, int dstStart)
>     Then you only need one method: outOfBoundsMsg() !!
>     Note that append(CharSequence s, int start, int end), calling
> getChars(), also uses start, end.
>
>
If a method has a "src" and "dst", then it makes sense to make that clear
in the parameter names, but if it only has a "src", then it makes sense to
leave out the prefix "src" in the parameter names.  It's annoying to use
the words "begin" and "start" interchangeably.  I prefer "start", but it's
probably not possible to fix this now.


> 4. Instead:
>         if (start < 0 || start > end || end > count)
>     you could write:
>         if (start < 0 || end - start < 0 || end > count)
>     because result of end - start could be reused later.
>
>
Yeah, and we can do even better:

    public void getChars(int srcBegin, int srcEnd, char[] dst, int
dstBegin) {
        int length = srcEnd - srcBegin;
        if ((srcBegin | length | (count - srcEnd)) < 0)
            throw new StringIndexOutOfBoundsException
                (outOfBoundsMsg(srcBegin, srcEnd));
        System.arraycopy(value, srcBegin, dst, dstBegin, length);
    }



> 6. insert() methods should likewise throw SIOOBE instead IOOBE.
>     Then likewise outOfBoundsMsg() could be used.
>
>
I'm willing to change exception detail, but not the actual class of the
exception thrown.