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/%7Emartin/webrevs/openjdk8/getChars/>

Alan et al: Before continuing, can we:

Have thumbs up on the changes to out of bounds exceptions?

This looks great, but AbstractStringBuilder could be again simplified ...

1. What is the difference between replaceOutOfBounds() and 
substringOutOfBounds() ? ;-)

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.

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.

5. One more abstraction:
    int checkBounds(int start, int end, int count) {
        int len = end - start;
        if (start < 0 || len < 0 || end > count)
            throw new StringIndexOutOfBoundsException
                (outOfBoundsMsg(start, end, count));
        return len;
    }

6. insert() methods should likewise throw SIOOBE instead IOOBE.
    Then likewise outOfBoundsMsg() could be used.

7.:
 504         int newCount = count + end - start; // remember in local variable
 505         ensureCapacityInternal(newCount);
 506         s.getChars(start, end, value, count); // can reuse result of end - 
start if GIT-inlined
 507         count = newCount ;


8.:
 819         int remainingStart = start + str.length();
 820         int remaining = count - regionEnd;
 821         int newCount = remainingStart + remaining;
 822         ensureCapacityInternal(newCount);
 823         System.arraycopy(value, regionEnd,
 824                          value, remainingStart, remaining);

9.:
1128         tail = count - dstOffset;
1129         if ((dstOffset < 0) || (tail < 0))
1130             throw new IndexOutOfBoundsException("dstOffset "+dstOffset);
1131         int len = checkBounds(start, end, s.length());
1132         int newCount = count + len;
1133         ensureCapacityInternal(newCount);
1134         System.arraycopy(value, dstOffset, value, newCount - tail, tail);
1137         s.getChars(start, end, value, dstOffset);
1138         count = newCount;


-Ulf

Reply via email to