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 StreamReader
,
you will get a FileNotFoundException
.
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