Introducing the for-if anti-pattern

Raymond Chen

Over the years, I’ve seen a bunch of coding anti-patterns. I figured maybe I’ll share a few.

Today, I’ll introduce what I’m calling the for-if anti-pattern, also known as “We’ll sell you the whole seat, but you’ll only need the edge.” This is a special case of the for-case anti-pattern, where all but one of the cases is null.

for (int i = 0; i < 100; i++) {
  if (i == 42) { do_something(i); }
}

This can naturally be simplified to

do_something(42);

The for-if anti-pattern arises in many forms. For example:

foreach (string filename in Directory.GetFiles("."))
{
    if (filename.Equals("desktop.ini", StringComparison.OrdinalIgnoreCase))
    {
        return new StreamReader(filename);
    }
}

This enumerates all the files in a directory looking for a specific one, and if it’s found, it returns a stream on it. The slightly-less-crazy version would be

if (File.Exists("desktop.ini"))
{
    return new StreamReader("desktop.ini");
}

Note that both versions of the code fragment have the same race condition: If the file desktop.ini initially exists but gets deleted before you get around to creating a new Stream­Reader, you will get a File­Not­Found­Exception.

One final example:

foreach (object o in hashtable.Keys)
{
    if (o == "target") return hashtable["target"];
}

Also known as

return hashtable["target"];

I bet these people hate going to the library to get a book by title, because it takes so darn long: They go up to the librarian and say, “Please give me all the books you have,” and then they fill up their cart with thousands of books, then sit in the corner saying, “Nope, the title of this book is wrong. Nope, not that one either. Still the wrong title. How about this book? Nope, not this one either. Man, this is taking forever…”

0 comments

Discussion is closed.

Feedback usabilla icon