Nice Code Structure

Here's a brilliant one I just ran across at work (variables changed to protect the... to protect someone):

  for (i = 0; i < lim; i++) {
    if (someCondition(n[i])) {
      warn("you can't do that");
      continue;
    }
    switch (n[i].type()) {
      default:
        nn = 0;
        warn("we're ignoring this");
        break;
    }
    orig->doSomethingWith(nn);
    delete nn;
  }

How do you like that switch with only a default branch? Pretty cool, huh? Almost is nice is the delete that only ever sees a NULL. Then when I realized doSomethingWith() is a no-op if it gets a NULL, I felt even luckier.

Look, if you're making such a big change to some code that you take out all the branches of a switch, isn't it worth a couple minutes to clean this up and show what is really happening? After all, the code really does this:

  for (i = 0; i < lim; i++) {
    if (someCondition(n[i])) {
      warn("you can't do that");
    }
    else {
      warn("we're ignoring this");
    }
  }

It probably makes sense to only give the second warning, since who cares if you can't do it, when really it would only be ignored anyway. And at that point you might wonder why you need to give the same warning lim times.

Ugly code kills. Kill ugly code.