This article is part of our ‘under the hood’ series, where we examine coding concepts and techniques related to how we build our products and services for our clients. If you’re of a technical bent, then do check out other articles as they appear in our ‘under the hood’ category.
One of the most useful aspects of RSpec and BDD is how it can highlight areas of your code that are obvious targets for improvement.
When we write our specs before our code we are effectively writing the API, the contract that our code must fulfill. It’s often the case that if this contract is hard to write, or just looks clumsy, then there may be a better way to express it. This makes RSpec our first way of detecting code smells.
Perhaps the easiest type of code smell to detect with RSpec is Law of Demeter violations. Imagine a simple Ruby on Rails task management application. Tasks can be owned by Users. There may be many places where we would want to show who owns what Task. The simplest way to do this in Rails would be to use a belongs_to association to expose the attributes of the User to the Task. This makes it trivial to get the name of the user who is assigned to a task:
class Task < ActiveRecord::Base
belongs_to :owner, :class_name => "User"
end
# to get the owner name:
task.owner.name
While this is easy to write, it would make our specs more complex. Anywhere we wished to mock the name of the owner we would have to write something like the following:
task.stub!(:owner).and_return(mock_model(User, :name => "Bob Smith"))
There’s a lot of noise in this line. We’re creating a mock object just to produce the name of the user. Typing and maintaining test code like the above just seems unnecessary — we’re mocking more than we need. Perhaps worse, it doesn’t express our intentions clearly. We’re asking the task to return its owner, then asking that owner to give us its name. Instead of this approach, we can define the contract as we go and we can actually express our intention:
task.stub!(:owner_name).and_return("Bob Smith")
This makes our tests look tidier, but it actually has a more beneficial side effect. Even though we now have to write more code (speccing then adding the owner_name method to the Task class), it’s now the Task’s job to find the name of its owner. This makes our code more maintainable. For instance, imagine that not every task has an owner. With our original code, anywhere we explicitly used the owner association we would have to change the code to:
(task.owner.nil? ? "No Owner" : task.owner.name)
If this was used many times throughout our code base, we’ve suddenly got a large number of places to find and test this code. By defining the owner_name method early we avoided this problem because there’s only one place that now has to deal with the messy business of finding the name of the owner. Our other specs and stories aren’t broken because they can still happily call the owner_name method without worrying about how this is actually handled.
By using RSpec to help us separate out what information we want from how we get this information, we’ve made our codebase just a little bit cleaner and more maintainable.
