Unless what?

You know, I really would love to have a heart-to-knife conversation with developers who use complex unless conditions. While trying to tease apart some code, I came across this gem:

unless ( $blocklist->has_block_with_id( $db_obj->id )
      || ( ! $allow_duplicates 
        && exists $episode_ids->{$db_obj->version->episode_id} ) ) {

To be fair, code grows over time and it's easy to understand how issues like this crop up (and this is a fairly old bit of code), but it took me a while to make sure I understood this. My naive conversion of this to an if statement failed. I had to reduce it down to its simplest components and do a truth table:

unless ( x || ( !y && z ))

     x y z    x yz    xyz   =
     0 0 0    0  0      0   1  
     0 0 1    0  1      1   0
     0 1 0    0  0      0   1
     0 1 1    0  0      0   1
     1 0 0    1  0      1   0
     1 0 1    1  1      1   0
     1 1 0    1  0      1   0
     1 1 1    1  0      1   0

And reversing that gives us:

if ( !$x && ($y || !$z)

     x y z    x yz    xyz   = 
     0 0 0    1  1      1   1
     0 0 1    1  0      0   0
     0 1 0    1  1      1   1
     0 1 1    1  1      1   1
     1 0 0    0  1      0   0
     1 0 1    0  0      0   0
     1 1 0    0  1      0   0
     1 1 1    0  1      0   0

Or:

if (
  ! $blocklist->has_block_with_id( $db_obj->id )
  && ( $allow_duplicates
    || !exists $episode_ids->{ $db_obj->version->episode_id } )
  )
{

I know that many of you would have no problem breaking down the conditional and have De Morgan's Law encoded in your DNA, but I'm not that smart. I still have to do it the hard way.

Please, take pity on us mere mortal programmers and don't use complex "unless" conditions!

5 Comments

You're not alone, I had to do a truth table and even a Karnaugh Map just to make sure I was thinking straight about a condition. :)

I often bemoan that De Morgan's is one of the few things I actually retained from my degree, but I guess I should be glad I have it. =-)

I've more or less reached the stage now (old age) where I end up extracting the meaning of anything with more than two simple clauses into a variable. Not sure if there's a name for them - "documenting variables"?


my $can_add_to_list = 
  ! $blocklist->has_block_with_id( $db_obj->id )
  && (
    $allow_duplicates
    || !exists $episode_ids->{ $db_obj->version->episode_id } 
  );

if ($can_add_to_list) { ... }

Who was it publicising "skimmable code" a while back? Ah, yes Mr Schwern - http://en.oreilly.com/oscon2008/public/schedule/detail/3011 I liked the idea, because it's exactly what you do with a new (or re-encountered) code-base so it makes sense to make it easy.

I try to make code directly reflect what a human thinks is going on. Our brains can do boolean logic, but they don't scale well at it.

On the other hand, Perl has named blocks and a rich set of loop control statements:

DO_IT: {
    last DO_IT if $blocklist->has_block_with_id( $db_obj->id );
    if ( exists $episode_ids-{$db_obj->version->episode_id} ) {
         last DO_IT if not $allow_duplicates;
    }
    .
    .
    .

Imagine adding additional conditions to the above (alas, the sort of thing that does happen in real life). I believe the solution by named block with loop control statement would scale reasonably well.

Jeffrey:

If you can keep all your lasts at the top level of the extra scope (your example didn’t), and can keep them all to very simple conditionals, then sometimes this is worthwhile. Because there is a name for inscrutable flow control: it’s called spaghetti code.

Personally I’d solve this particular example in the way Richard suggested, but would use extra variables not to encapsulate the entire conditional, but to give names to its parts.

my $is_blocklisted
     = $blocklist->has_block_with_id( $db_obj->id );

my $is_rejected_dupe
     = (not $allow_duplicates)
     and exists $episode_ids->{ $db_obj->version->episode_id };

return if $is_blocklisted or $is_rejected_dupe;
# or
unless ( $is_blocklisted or $is_rejected_dupe ) { ... }

Yes, a return to unless. Yet I bet no one who read this had any trouble whatever understanding the conditional.

Leave a comment

About Ovid

user-pic Have Perl; Will Travel. Freelance Perl/Testing/Agile consultant. Photo by http://www.circle23.com/. Warning: that site is not safe for work. The photographer is a good friend of mine, though, and it's appropriate to credit his work.