2006-09-13

An Old Friend

Last night after posting, I went to visit a friend I haven't seen in about five years. He was pretty much my best buddy in High School and lived right across the street from me. Sadly, he's one of those people that hasn't "broken his mold." He's still looking for a better job, better place to live, while still playing video games late into the night. I hope he finds what he's looking for and soon.

While I finally got Evolution to get the right time on my PDT appointment (I couldn't edit the time using the interface, and changing the time/timezone/combo on the actual .ics file didn't help either so I just deleted it and made my own). The honor of today's code complaint goes to the guy that's posting tomorrow's www.thedailywtf.com, Eric Sink. He had a blog posting that made reddit because he was able to make 100% code coverage (http://software.ericsink.com/articles/Code_Coverage.html). While this is a commendable feat, there is something that just isn't right about his example:


if (condition1)
{
return result1;
}
else if (condition2)
{
return result2;
}
else if (condition3)
{
return result3;
}


Here's a hint, it isn't the fact that I left off the exception part he had so that he would get past the "not all control paths return a value" message. It's the fact that if he wrote the code right he would only need one return value.

Think of the poor sap that uses this function five years down the road and discovers it doesn't work with his exciting new app. How much harder is it going to be for him to find the bug when he's putting a command to debug what is returned by the function with it laid out that way? Compare it to this:


returnValue = INVALIDRETURNVALUE;
if (condition1)
{
returnValue = result1;
}
else if (condition2)
{
returnValue = result2;
}
else if (condition3)
{
returnValue = result3;
}
return returnValue;


It's simpler to follow, prevents the C# error, and allows for easy use later down the road. This is what proper software engineering is about. Not just getting it working, but making it right for the people that extend/maintain/etc. the code after you.

No comments: