2
\$\begingroup\$

Something that has bugged me for a while, is what way is best to perform a conditional where a string is compared to other strings.

Here is some sample code. This code is just for illustration purposes.

The 3 methods I use are

  1. Do a word for word comparison. This is probably the most optimal, but ugliest code
  2. Create an array on the fly and do a comparison
  3. Have a predefined array. This is probably better placed in an external file for easier maintainability.

I just wanted to see what your takes are on this, and if there is an even better way to do it. I know Enums would probably work well here as well.

    private bool IsWordBad(string word)
    {
        //First Method
        if (word == "Foo" || word == "Bar" || word == "FooBar")
        {
            return true;
        }


        //Second Method
        if ((new[] { "Foo", "Bar", "FooBar" }).Contains(word))
        {
            return true;
        }

        //Third Method
        var badWords = new[] { "Foo", "Bar", "FooBar" };
        if (badWords.Contains(word))
        {
            return true;
        }
        return false;
    } 
\$\endgroup\$

3 Answers 3

4
\$\begingroup\$

If you really have just three words, then the performance most likely won't matter. In other words, unless profiling shows that this method is performance-critical, use the method that you find most readable.

Some other options you could consider:

  1. Using a switch:

    switch (word)
    {
        case "Foo":
        case "Bar":
        case "FooBar":
            return true;
        default:
            return false;
    }
    

    This has the advantage that it can use compiler optimizations. (Under certain circumstances, the compiler will turn such switch into a Dictionary lookup.)

  2. Use a HashSet<string> instead of an array. Looking up in a hash set is O(1), but O(n) for an array. This could make sense if you had large number of strings to search for.

  3. Use a trie. This has the advantage of comparing only relevant parts of the input string. This could make sense if you had long input strings or if you huge number of strings to search for.

\$\endgroup\$
3
  • \$\begingroup\$ Not sure a HashSet has an O(1) lookup time; it would mean the hash used is actually a bijection... \$\endgroup\$
    – fge
    Jun 5, 2013 at 17:51
  • \$\begingroup\$ @fge That's not true. O(1) lookup doesn't require no collisions, all it requires is that collisions are rare enough and don't slow you down too much. So, yes, in the worst case where all strings are in the same bucket, you'll get O(n) lookup, but that case is extremely unlikely. Under normal circumstances, HashSet really is O(1). \$\endgroup\$
    – svick
    Jun 5, 2013 at 17:57
  • \$\begingroup\$ @fge just to confirm that HashSet lookup is really a O(1) operation: see remark in HashSet<T>.Contains documentation \$\endgroup\$
    – almaz
    Jun 6, 2013 at 14:15
1
\$\begingroup\$

I'd use the third version, albeit with a small modification:

return badWords.Contains(word);

After all, the Contains() method returns a boolean!

Also, I'd put badWords out of the method, so that it needn't be recreated on each method call...

\$\endgroup\$
1
  • \$\begingroup\$ Are you able to clarify what you mean by "put badWords out of the method", as just because it's out of the method doesn't mean it might not be created on each method call. \$\endgroup\$
    – dreza
    Jun 5, 2013 at 20:05
0
\$\begingroup\$

also you can try:

if(Array.IndexOf(badWords,word)==-1)
{
    //Your logic here 
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.