Eliminating Code Complexity by Finding Double Negatives

I have been reading “A Philosophy of Software Design,” and almost the entire book is spent either explaining the dangers of complexity in a codebase, or eliminating complexity.

Code, especially Ruby code, should make its intent loud and clear to the reader. Clear code is the opposite of complex code. It’s easy to understand its purpose, so it’s easier and less risky to modify, extend, or remove.

Double Negatives

One thing that I haven’t seen explicitly discussed in this book is double negatives.

Whether it’s spoken or written, double negatives make sentences less clear. Something like “There are not no eggs left at the store” is obviously poor grammar, but it also makes the sentence harder to understand.

When our code reads like a sentence (well written Ruby should), double negatives increase complexity. Things like unless !some_condition can be more clearly written as if some_condition.

Most programmers will find writing in-line conditionals like this to be straightforward, but it gets less intuitive when extracting this logic into methods.

An example

A bit ago, I was writing some ruby code with a friend. We wanted to take an action if something was false, and like the good ruby programmers we are, we extracted that complex conditional into its own short method.

We found ourselves going around and around trying to make this as clean as possible. One of our first iterations contained a programming double negative - see if you can find it:

# Method call
return [] unless items_found(users)


# Method implementation
def items_found(users)
  no_items_found = false
  users.each do |volunteer|
    if volunteer.items.present?
      no_items_found = true
    end
  end
  no_items_found 
end
Contrast this to what we ended up with
# Method call
return [] if no_items_found_for(users)


# Method implementation
def no_items_found_for(users)
  no_items_found = true
  users.each do |volunteer|
    if volunteer.items.present?
      no_items_found = false
    end
  end
  no_items_found 
end

It’s a pretty small change, but I think it highlights what the code actually does much better than the first time around. I’m trying to be more intentional about writing my code with these things in mind.

Comments

[Oliver]

I think you've moved in the right direction but the second implementation still makes my head hurt. How about something like this?

# Method call
items_found_for(volunteers)

# Method implementation
def items_found_for(volunteers)
  volunteers.reduce([]) do |items, volunteer|
    next items unless volunteer.items.any?
    items << volunteer.items
  end.flatten.uniq
end

(Perhaps also with the flattening and removing of duplicates handled within the reduce block to avoid the intermediate arrays.)

Now there's no need to explicitly return the empty array because items_found_for will return an empty array if there's nothing to add to it.