Taking Style Tips from Natural Language
When one speaks of the readability of a computer program, they refer to the ease with which the source code is read by a human. More recently, with languages like Ruby and Python, one often hears praise for sections of code that seem to read like natural language. That sort of code is easier read, but seems to be more challenging to write. If english-like is what we're after, perhaps style manuals can teach us something about writing code.
Having always been interested in writing English, I've read several books on style. The text that I continue to return to is Strunk & White's The Elements of Style. It has served me well as a reference for writing English; let's see how it does for Ruby.
Lesson One: Put statements in positive form.
The rule from Strunk & White:
Make definite assertions. Avoid tame, colorless, hesitating, noncommittal language. Use the word not as a means of denial or in antithesis, never as a means of evasion.
Elements of Style provides a short explanation of each rule, followed by several illustrative examples. Each example contains a violating passage, and a correction. Here's an example from this rule...
The violating example:
He was not very often on time.
...is corrected by:
He usually came late.
Yes, Strunk and White provide refactorings; I'll do the same.
if !@post.save
render :action => 'create'
else
redirect_to post_url(@post)
end
That example is rather simple. It says: If the post doesn't save, render the create action, otherwise, redirect to the post's url. I think we can agree that in most cases, it would be better to express that as follows:
if @post.save
redirect_to post_url(@post)
else
render :action => 'create'
end
By putting our statements in positive form, we have refactored that code to read: If the post saves, redirect to its url, otherwise render the create action. Because we have eliminated the negation, the code reads more smoothly and easily. While that is an obvious example, it shows that there is something to this. Let's look at another rule.
Lesson Two: Use the active voice.
The rule from Strunk & White:
The active voice is usually more direct and vigorous than the passive.
The violating example:
It was not very long before she was very sorry that she had said what she had.
The correction:
She soon repented her words.
This rule doesn't apply as directly, but I have always loved it as a style guideline for code. Take this rather common example:
[1, 2, 3].each do |number|
if number.even?
# do something with an even number
end
end
That example says: With each element in this array, if that element is even, do something with the even element. Collecting the desired elements that way reads passively. Making use of some more of our tools from ruby's Enumerable module, we can refactor it to use the active voice.
[1, 2, 3].select(&:even?).each do |number|
# do something with an even number
end
The refactoring says: With all of the even numbers in this array, do something. It reads shorter, and expresses the intent of the coder more directly.
From the Real World
The next example we're going to look at comes from ActionController::Base (a central class in rails). I have selected a few illustrative branches from a conditional that spans some sixty lines, in a method that spans nearly one hundred. It is the rendering logic:
if file = options[:file]
render_for_file(file, options[:status], options[:use_full_path], options[:locals] || {})
elsif template = options[:template]
render_for_file(template, options[:status], true)
elsif inline = options[:inline]
add_variables_to_assigns
render_for_text(@template.render_template(options[:type], inline, nil, options[:locals] || {}), options[:status])
elsif action_name = options[:action]
template = default_template_name(action_name.to_s)
if options[:layout] && !template_exempt_from_layout?(template)
render_with_a_layout(:file => template, :status => options[:status], :use_full_path => true, :layout => true)
else
render_with_no_layout(:file => template, :status => options[:status], :use_full_path => true)
end
end
It says (bear with me): If the file option is present, render the file, or if the template option is present, render the template, or if the inline options is present, pass the instance variables to the template, and render some inline text from the controller, or if the action option is present, render that action. Note: that transliteration has been shortened somewhat.
The passiveness of a multi-branch conditional damages clarity; it creates a meandering experience for the reader. The problem is that separate concerns are being forced together. In the case of the render method, the conditional determines which type of rendering to perform, and contains the actual code to perform that type of rendering.
Long conditionals also seem to resemble run-on sentences. Since one must remember all of the context as they read through the branches of this sort of structure, it quickly becomes difficult to follow. It would be better to separate the decision from the specific rendering logic; that will make it easier to be more direct.
def render
render_types = [:file, :template, :inline, :action]
type_to_render = render_types.detect { |render_type| options[render_type] }
send(:"render_#{type_to_render}")
end
def render_action
template = default_template_name(action_name.to_s)
if options[:layout] && !template_exempt_from_layout?(template)
render_with_a_layout(:file => template, :status => options[:status], :use_full_path => true, :layout => true)
else
render_with_no_layout(:file => template, :status => options[:status], :use_full_path => true)
end
end
def render_inline
add_variables_to_assigns
render_for_text(@template.render_template(options[:type], inline, nil, options[:locals] || {}), options[:status])
end
def render_template
render_for_file(template, options[:status], true)
end
def render_file
render_for_file(file, options[:status], options[:use_full_path], options[:locals] || {})
end
Now, it reads: The render types are file, template, inline, or action. The type to render is the one that is present in the options hash. Call the method named: render underscore the type to render. Refactored, each of the ways of rendering gets its own method, and those methods have a name. That way, it is clear what each of those blocks of code does, and what we're doing when we call it; we needn't decipher a conditional to find our way to the pertinent logic. Separating code in to small chunks housed by well named methods allows us to express our intentions directly.
As an aside, writing methods this way is far more testable. In its original form, the render method does many different things, depending on the parameters it receives. Testing it would be an exercise in managing side effects. You'd also be testing two different things at once: does it reach the correct branch of the conditional, and does it execute the rendering properly. With the simple refactoring above, you can test the rendering logic separately from the selection of which type of rendering to perform. Dividing methods in to smaller pieces nearly always allows you to isolate functionality more effectively.
And There's More...
I've only scratched the surface in this article. There is a lot more to be learned from natural language writing style. I highly recommend picking up a copy of The Elements of Style. Worst comes to worst, you'll greatly improve your writing of English; best case scenario, you'll pick up a lot of insightful tips on writing clearer and more readable code.