Postfix conditionals and language design

[dougo]

So I'm reading some Perl code. I usually stay away from Perl as much as I can, but I'm trying to figure out some undocumented behavior of the MusicBrainz RDF server so I have to UTSL. Anyway, I get to this line:

      return ("No artist name or artist id given.", $data, 0, undef) 

and I get confused, because I saw the artist ID check but not the artist name check. So I start reading backwards to see if I missed something; maybe the artist name check happened in some non-obvious function call? I go skimming through various other files to see if some other code is filling in the artist ID or something. Eventually, I give up and come back to that line, and I notice that the next line is:

          if ($data->{artist} eq '');

and I smack my head. I'm willing to chalk this up to bad style (and I hope people would agree this is bad style), but now I'm wondering: when is it ever good style to put the conditional guard after the code it's guarding? Or, more generally, when is it ever good style for the order of code (in the same basic block) to be the opposite of the order of execution?

Comments

[novalis.livejournal.com]

It probbably was originally shorter, such that it fit on one line. Then it would be better style (as noted in petdance's comment).

[novalis.livejournal.com]

I guess I have to disagree with this -- "next if" and "return if" give a net savings of two lines, and while brevity shouldn't be chosen at the expense of clarity, it's generally a good thing. If you're editing code, you have the responsibility to make it clean, just as you do when you're writing it.

[dougo.livejournal.com]

It's only two lines if you use two lines—or does Perl enforce line breaks or something?

[novalis.livejournal.com]

No, it's only two lines if you put in a line break. But otherwise, at least with indent, it's so long that it wraps.

[dougo.livejournal.com]

I was actually thinking about amending the basic-block thing after I posted, but I figured no one would care. :) What I meant was expressions in the same "leaf" procedure, i.e. not in different procedures that happen to be inside the same procedure.