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

Martin Buchholz martinrb at google.com
Sun Mar 21 07:56:53 UTC 2010


On Sat, Mar 20, 2010 at 14:52, Ulf Zibis <Ulf.Zibis at gmx.de> wrote:
> Am 20.03.2010 01:13, schrieb Martin Buchholz:
> Yep, I stepmotherly revised other methods, my focus was on String(int[],
> int, int) and outsourcing bond checks.
> BTW, what do you think of the latter?

I've done a lot of work on "out-lining" error handling.
The state of the art appears to be (from LinkedList)


    /**
     * Tells if the argument is the index of an existing element.
     */
    private boolean isElementIndex(int index) {
        return index >= 0 && index < size;
    }

    /**
     * Tells if the argument is the index of a valid position for an
     * iterator or an add operation.
     */
    private boolean isPositionIndex(int index) {
        return index >= 0 && index <= size;
    }

    /**
     * Constructs an IndexOutOfBoundsException detail message.
     * Of the many possible refactorings of the error handling code,
     * this "outlining" performs best with both server and client VMs.
     */
    private String outOfBoundsMsg(int index) {
        return "Index: "+index+", Size: "+size;
    }

    private void checkElementIndex(int index) {
        if (!isElementIndex(index))
            throw new IndexOutOfBoundsException(outOfBoundsMsg(index));
    }

    private void checkPositionIndex(int index) {
        if (!isPositionIndex(index))
            throw new IndexOutOfBoundsException(outOfBoundsMsg(index));
    }

Study also:
http://code.google.com/p/google-collections/source/browse/trunk/src/com/google/common/base/Preconditions.java


> - A little "bug" in javadoc:
>  @exception ArrayIndexOutOfBoundsException
>  instead    IndexOutOfBoundsException

Not a bug.
You do realize AIOOBE is a subclass of IOOBE?

> - String#indexOf(int, int):
>             // handle supplementary characters here
>             char high = Character.highSurrogate(ch);
>             char low = Character.lowSurrogate(ch);
>             for ( ; i < max-1; i++)
>                 if (v[i] == high && v[i+1] == low)
>                         return i - offset;

I now believe we should provide
Character.highSurrogate and Character.lowSurrogate
as you have been advocating.

If Sherman agrees, let's put a proper patch for this together.

Martin



More information about the core-libs-dev mailing list