Wednesday, March 30, 2011

A couple of things Developers Should Know but often Don't

I've been reviewing a lot of code lately. Mostly Java code, but some C, some C++ event some Objective C.  There have been some patterns to the coding mistakes/miss-steps that I've seen and been pointing out in a number of these review.

Loops
This one seem to be the most common of late. And I had to convince some people of the merit of my stance.  Basically my theory is that you should almost never need a while loop.  So if you ever see yourself using one.  Ask yourself why.  Most times you'll be declaring some kind of iteration tracking variable and incrementing or updating it somehow as you loop through whatever structure is being iterated over.

The problem there is that the variable gets declared and used outside the loop.  Now C doesn't help here so it's common for C developers to make this mistak in languages that do help. In my case Java.  Using a for loop allows you to declare a variable that is scoped to you loop so you can be sure you don't accidentally keep the variable longer than you need and use it by accident somewhere else in your function.

I think the latest C standard (C99) fixes this, but it's pretty common that you can't count on that so a lot of C developers still don't think about this feature.  Just about the only time I think a while loop is a bit cleaner or clearer is if you are creating an infinite loop; in that case the simple while(true){} is pretty universal.  Although I've seen for(;;){} a number of times as well so I'm still not sold on that.

Proper Scope
Along the same lines as my previous issue with loops is ensuring that your variables are scoped properly.  I hate seeing Java functions with a long, long laundry list of auto variables that are potentially not needed since the the parameters may make their allocation unnecessary.  Although this isn't really a memory or optimization effort it does help a bit in that respect.  The Java stack frame will be allocated with space for all the local variables.  Event the objects, although only space for the reference is allocated by default.

So although you will not be preventing local variables from being allocated, if you have the the objects you create properly scoped they will at least only do the object allocation when needed and will be dereference and available for GC as soon as possible.  But the optimization can come into play if your objects are only going to be created if certain parameters are passed into that method.  So in cases where your parameters don't require the objects to be created you save that time and memory.  But like I said, this isn't really the reason to scope them properly.

The best reason for scoping variables is for clarity and maintenance. The issue with keep a variable around longer than you need to is that it is unclear if you intended to use it later and just forgot, or worse yet you do use it later, but didn't actually mean to do so.  Don't laugh.  This happens a lot.  In particular in long, hairy functions.  I don't like those either, but baby steps.


Over use of if this set this else set that
I think this is a pretty easy one.  Which would you think is clearer and more concise:
Option 1:
if (conditiona) {
   var = boo;
}
else {
   var = bar;
}

Option 2:
var = (conditional) boo : bar;

Option 2 is the clear winner in my books hands down every time.  I think this is the very reason why the ternary operator gets included in pretty much every language I've had the pleasure of working with.

I have lots of other gripes and faux paus that I see on a daily basis, but these are just some of the most common that I've seen of late.  One of my biggest pet peeves is when OO design gets perverted into some crazy mess of over complicated goo.  Often that seems to start with good intentions and the classic over use of inheritance rather than composition.  I've trained myself similar to the for loop to stop every time I see inheritance to ask if it is actually the most appropriate solution to the problem at hand and if it truly adds value beyond what composition could provide.  I feel like I'm stating the obvious since I've been reading about this stuff on blogs like this for years, but I keep seeing these same issues in the code I look at being created on a daily basis so people still aren't getting the message.  I figured I may as well add to the ocean of data out there pleading with developers to clean things up.  But I'm sure it's a Sisyphus like task. But I'll keep pushing that damn rock anyway.   It's just how I roll.

1 comment:

  1. A comment was made to me via Buzz that while loops have a particular use when it comes to doing algorithmic proofs where you have strict pre/post conditions that need to be validated.

    I can't say that I've done a whole lot of that type of development, if any. And the example came from work done in academia. My response to that point is essentially this:

    My point wasn't that while loops should never ever be used. Only that they aren't normally the best choice due to the variable scoping issue. My suggestions was to just ask why you were using it. If you can answer that question with good sound reasons, then you're doing the right thing. Almost nothing in software development is black and white.

    ReplyDelete