I Am Not Myself

Bills.Pay(Developer.Skills).ShouldBeTrue()

Pet Peeve Code Smell: Multiple Calls to a Method in a Block

I was just reading Ayende’s post on coding pitfalls and it’s associated comments. This struck me as less about pitfalls and more about pet peeves. A personal pet peeve of mine is when I discover a block of code that makes multiple calls out to fetch a single value while processing.

For example:

if(Call.ToSomethingByName("Name") != "foo") {
  if(Call.ToSomethingByName("Name") !="bar")
    var something = Call.ToSomethingByName("Name");
    processNonFoos(something);
  else
    var someotherthing = Call.ToSomethingByName("Name");
    processFoos(something);
}

Just seeing this kind of construct tells me the person who wrote it was not thinking about anything other than “make it work”. They were so focused on getting the task at hand done and moving on they didn’t stop to think about the all the possible gotchas and potential failure cases.

This code was most likely tested by hitting F5, plugging some text into a form and clicking submit. As long as no errors were thrown the developer moves on to the next task.

Just looking at this code, how many possible failure points do you see? What happens if Call.ToSomethingByName(“Name”) return null? What happens if Call.ToSomethingByName(“Name”) makes calls to the database? What happens if Call.ToSomethingByName(“Name”) returns something different on each call?

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: