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.