Thanks,
I'll open an issue and create a patch
On Sun, Dec 28, 2008 at 5:55 PM, Michael McCandless <
luc...@mikemccandless.com> wrote:
>
> I think all of these changes make sense!
> I would just remove that if (false &&...) dead code.
>
> Mike
>
>
> Shai Erera <ser...@gmail.com> wrote:
>
>> 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
>>
>
>
