Missing Perl::Critic plugins I: RequireElseAfterElsif

jplindstrom on 2008-04-01T17:06:23

So this is a bug waiting to happen. Or at least a potential problem.

if ($period eq 'a') {
    ...
}
elsif ($period eq 'c') {
    ...
}
elsif ($period eq 'd') {
    ...
}


Why?

Well, if none of the if statements are hit, what should happend? Is it okay to omit that case, or is that in fact a bug because something didn't get handled?

Without reading and understanding the code you can't know whether the lack of an else clause is intentional or accidental. So it doesn't communicate intent clearly at all.

An empty else clause may seem redundant here. And it is redundant. To the compiler. But it communicates to the maintenance programmer: yes, I actually thought if this case too.

But, my experience is that whenever there is an elsif chain without a final else clause to catch all remaining cases, that is often a bug. The missing else clause leads to something not happening at all.

Sometimes the missing else clause is not a bug right now. One of the elsif clauses will always hit given the current input. But if anything related to producing the input changes (somewhere in a distant corner of the code base, far away) then this piece of code suddenly breaks. Silently.

At the very least, there should be a guarding else clause which says

die("Internal error, sanity check failed: blah blah blah");


So, dear Lazyweb, please write Perl::Critic::Policy::ControlStructures::RequireElseAfterElsif for me.

Or else.


What about RequireElseAfterIf?

Aristotle on 2008-04-01T18:19:28

Doesn’t every if need an else, regardless of any elsifs?

Re:What about RequireElseAfterIf?

jplindstrom on 2008-04-01T21:40:48

Why?

Is it your experience that if without an else is usually a problem?

Re:What about RequireElseAfterIf?

Aristotle on 2008-04-02T08:26:09

No. It’s also not my experience that an elsif chain without an else is commonly a problem. So I’m wondering if you are seeing a systematic problem with conditionals – and if not, what the root issue really is.

I wonder if it has something to do with the fact that when I intentionally write elsifs with no else, there are usually only one or two elsifs. (But again, this is usually.)