Hi

I noticed that CharArraySet uses Character.toLowerCase in many places in the
code. I think it uses it too much unnecessarily.
For example, the equals(char[], int, int, char[]) method converts the
characters to lower case if ignoreCase is true, although it could have been
converted in one of the appropriate public APIs (like add or contains).
Moreover, the equals() method may be called several times during the same
add/contains call, and the value will still be lowercased every time.

The same logic cannot be applied on the CharSequence contains API though,
for efficiency reasons. The rest (including all the add() methods) can be
changed to first convert the value to lowercase form (if ignoreCase is
false) and then simplify getSlot() and equals() by removing the ignoreCase
check from them.

What do you think? I can prepare a patch if you agree to make this change.

Also, few minor changes we can push through in this patch are:
One
------
add(Object) looks like this:
    if (o instanceof char[]) {
      return add((char[])o);
    } else if (o instanceof String) {
      return add((String)o);
    } else if (o instanceof CharSequence) {
      return add((CharSequence)o);
    } else {
      return add(o.toString());
    }
I don't think there's a need to check for instanceof String. Instead it
could be written simply like this:
    if (o instanceof char[]) {
      return add((char[])o);
    }
    return (add(o.toString());
The reason is that add(String) and add(CharSequence) eventually do the same.
If the identification of CharSequence is still important, it can be written
like this:
    if (o instanceof char[]) {
      return add((char[])o);
    } else if (o instanceof CharSequence) {
      return add((CharSequence)o);
    } else {
      return add(o.toString());
    }

Two
------
getHashCode(CharSequence) has this check:
      if (false && text instanceof String) {
        code = text.hashCode();
Why is it there? It will always fail.

Shai

Reply via email to