00001 December

Please stop throwing blank? at everything.

There’s a very cool principle I’d like to introduce you to: ‘Code as documentation’.

If you have any favourite programming blogs, you’ll probably have read enough decent code snippets to have figured this out yourself. In a nutshell: I should know what your code does by reading through it.

I’ve found that many of the code snippets I’ve seen toted as examples of ‘good style’ read very easily.

Good code should convey as much information as possible while putting as little strain on the reader as possible. Easy reading is damn hard writing.

In the end, it comes down to simplicity and effective communication.

ActiveSupport added the #blank? method to various core classes to simplify your life. Let’s see what #blank? can do for you:

some_array = nil
some_array.blank?
#=> true

some_array = []
some_array.blank?
#=> true

some_array = [nil]
some_array.blank?
#=> false

Pretty cool! You say. If an array/hash/string is nil or empty ([], {}, ” “, respectively) we can test for both cases with #blank?.

I like #blank? because it allows us to get closer to the principle of Code as Documentation. No more cumbersome members.nil? or members == ""!

However, it’s easy to fall into the trap of writing members.blank? instead of members.nil?. Why do I call this a trap?

Let me explain. When reading through source code when jumping on board with an existing project, you’re building connections between the various parts of the system. Now read the following and tell me which is easier:

def method_one(param_1)
  param_1.map do |e|
    e.some_calculation
  end unless param_1.blank?
end

Let’s look at that method. We receive a parameter (param_1). We then call #map on it, meaning it is enumerable. Since only one block argument is bound, we assume it’s not a Hash and we finally conclude that it’s probably an Array.

So method_one maps over some array we pass to it, collecting some_calculation and returning that unless the array we pass it is blank?.

WOAH! So that method means:

def method_one(param_1)
  return nil if param_1.nil? or param_1.empty?

  param_1.map do |e|
    e.some_calculation
  end
end

The first thing that bugs me about this method is that as I’m still reading through the codebase, I don’t know all the places method_one gets called. I have to assume that the author meant ‘sometimes I want to pass nil to method_one and sometimes I’ll pass []. OK, in my mental picture of the codebase and how everything fits together, I remember that method_one sometimes takes nil arguments and other times gets passed [] as argument.

This is all fine if the author really meant that method_one should accept both types of argument (Array and Nil) and if param_1 is either [] or nil we should return specifically nil.

However! Far too often these criteria don’t hold:

  • It turns out we never pass nil to method_one. This results in the following simplification:
def method_one_1(param_1)
  return nil if param_1.empty?

  param_1.map do |e|
    e.some_calculation
  end
end

(Besides, we should being returning false here or :failed to convey even more information to both the calling method and the reader!)

  • And it’s even more rare to return nil when the caller expects the method to map over an array! Returning [] is probably perfectly OK from the caller’s perspective:
def method_one_1(param_1)
  param_1.map do |e|
    e.some_calculation
  end
end

With two reasonable assumptions that I’ve found hold 90% of the time when working with #blank? we’ve massively simplified the reader’s life.

In summary:

if some_var.empty?
  do_something!
else
  do_something_else!
end

Tells me that if some_var is [] or {} when the program flows past this point, handle that fact, otherwise proceed as planned. From the context it should be apparent whether some_var is an Array or a Hash. As such, someone reading the code immediately knows that “ok, I know some_var is a list of Some objects” and proceed with that assumptions, reading the rest of the method.

if some_var.blank?
  do_something!
else
  do_something_else!
end

Says that at this point, some_var is one of nil, [], {}, " " and if that’s the case we need to do_something! otherwise we can proceed as planned… We still don’t have a clue what some_var is and every time it gets used in the rest of the method we need to try to figure out what type of object it’s supposed to be.

It should seem obvious that using #nil? or #empty? says a lot more in the same amount of calls as #blank? does.

So please, when you mean #nil?, use it! When you mean #empty? use that! And if you mean some_var.nil? or some_var.empty? or some_var.nil? or some_var == "" use #blank?!!! :)

Check out these