Case-insensitive String search in Array

Question

Wrote this function for a Stack Overflow answer. It started off pretty bad but in the end I polished it a bit and now I have something I think to be pretty clean. Actually, when I look at the code, this is what I usually write when I'm programming AS3 (minus the documentation, I'm a lazy bastard, although that's slowly changing too).

The problem the code attempts to solve is finding a case-insensitive string in an array.

Soo... anything that can be improved here?

Should I handle null and undefined?

Does my documentation look helpful from a IDE usage perspective?

/**
 * Searches through an array for a case-insensitive string match.
 * Attempts to imitate Array.indexOf where it can.
 * @param arr The array to search through
 * @param searchingFor The string to search for
 * @param fromIndex Optional, an index to start searching at.
 * @returns The index of the array that a match was found at (zero-indexed), or -1 if no match was found.
*/
public static function indexOfCaseInsensitiveString(arr:Array, searchingFor:String, fromIndex:uint = 0):int {
    var lowercaseSearchString:String = searchingFor.toLowerCase();
    var arrayLength:uint = arr.length;//Retrieving array length is expensive, optimization

    for(var index:uint = fromIndex; index < arrayLength; index++) {
        var element:* = arr[index];
        if(element is String && element.toLowerCase() == lowercaseSearchString) {
            return index;
        }
    }
    return -1;//It wasn't found in the array.
}

Answer 1

Documentation

/**
 * Searches through an array for a case-insensitive string match.

Excellent!

---

 * Attempts to imitate Array.indexOf where it can.

I'm not quite sure what you want to say with this.

---

 * @param arr The array to search through

There's no need to shorten the variable name a few characters. array is a better name, but still says absolutely nothing about what it is used for. In this case, I actually have to say that I like the variable names that PHP uses. haystack. Can it be any clearer? Nearly everyone knows about the saying "Finding a needle in a haystack". So the name haystack is a quite good one.

 * @param searchingFor The string to search for

Using the needle-haystack analogy, this can be called needle.

---

 * @param fromIndex Optional, an index to start searching at.

From an IDE perspective, I think it is not necessary to state that the parameter is optional. All ActionScript IDEs that I know of have built-in features for this, by stating parameter = defaultValue. Your documentation doesn't state what the default value is, so the IDE is more helpful here than your documentation. Of course though, one would expect it to start at index 0. I would follow how others have done and simply state "index to start searching at." Or, even better "The location in the array from which to start searching for the item."

---

 * @returns The index of the array that a match was found at (zero-indexed), or -1 if no match was found.

First of all, everyone who uses arrays should be aware that array indexes in AS3 is zero-indexed. Secondly, the term 'index' was also mentioned in the previous documentation line but there was no stating there that it was zero-indexed. I would leave the (zero-indexed) part out. The rest is perfectly fine, and is what one would expect out of the function.

Code

I would prefer a space after each colon, but that's just my personal preference. At least you are consistent about it.

It's great that you use uint in your code and not int or some other, shorter, type. A minor problem would be if the match was found at index >= 2147483648 though, as you return an int. This is however the same in many other indexOf functions because you want to return -1 if no match was found, and I doubt that you will get an array of length > 2147483647 as input. Also, retuning Number would be horribly overkill so there is really no other option here.

I personally find your comments in the code unnecessary, but they are definitely not "clutter" so they are fine. Although //It wasn't found in the array. can be removed as that behavior is described in the documentation of your method.

Now, there's only one thing left to say; Why not use the Vector.<String> type? Then you wouldn't have to check element is String for each element. Also, IIRC, Vector is faster than Array.

Overall, well done. Your code looks quite fine overall.

Null and undefined

You explicitly ask this question:

Should I handle null and undefined?

I would recommend that you at the beginning of the method check if the array or the search string is null or undefined and that you throw an error with a clear message if that is the case. It is often frustrating when a method throws an error because something is null, but as it is automatically thrown, it is harder for the programmer to find out what is null. Checking for preconditions at the start of the method is good practice!