Getting Rails on Track - Part 1: Models

In this multi-series blog post, we're going hands-on through the steps that can help transform the design of a default Rails application into one with clearer responsibilities and that is easier to test.

Have a look at the other parts, too!

You can follow along with a repository available on GitHub that contains all the code discussed here.

Active Record is the default persistency framework that comes with Rails. Using it makes it easier for us to design the data model of our applications. Reading and writing data is relatively simple, but the simplicity of Active Record's extended features shadow some of its pitfalls. These pitfalls do not surface for smaller applications, but can have an impact on our productivity as our application grows bigger.

For example, we can add custom validations whenever we want to save a record to the database. What starts as an innocent use of a feature can turn into a high-cost refactoring if we need to change the definition of what it means to be a valid record.

For that, let's have a look at the Movie class from the example application.

class Movie < ActiveRecord::Base
  validates :title, presence: true
  validate  :release_date_is_after_1895

  private

  def release_date_is_after_1895
    if release_date.nil?
      errors.add(:release_date, "must be be provided.")
    elsif release_date.year < 1895
      errors.add(:release_date, "is older than the world's first motion picture.")
    end
  end
end

Whenever a new movie gets persisted, Active Record will ensure that it has at least a name and that the release date is not before 1895. So far so good. What are the issues with the above code?

The simplicity of validates :title is a great feature of Active Record, as it allows a simple and concise way of expressing the need of a title. For those models whose requirements change frequently, this can cause trouble, though. Especially in a test suite that creates a lot of movies. Every time we change the definition of what it means to be a valid movie, there's a chance it has a rippling effect across our code base. This is also what the code smell “shotgun surgery” identifies.

To a certain extent, gems like factory_girl can help alleviate these issues. But before we go on and pull in more dependencies to fight symptoms, we should think about what the actual cause of the problem is.

The problem just described is not only visible in the test suite of our application. The same behaviour would be visible if a user wants to edit a program and save it without making any changes. All of a sudden a validation message could appear telling the user that the existing program she just edited is not valid anymore. While it is good that the validations work as expected, it's surprising and unexpected from a user experience perspective.

Alternatives

The tension between ease of use of a framework feature and maintainability stems from tightly coupled concerns about persistency and data validations. To find an alternative to that, we can ask ourselves what we usually do when things are too coupled together: we separate them.

Once we have one thing that deals with persisting a movie and another thing to validate a movie, we have done more than created better separated concerns. We also end up with two new classes that adhere better to the single responsibility principle. As a side effect, we get the benefit of better testability. Any test that's not specifically concerned about a valid movie can just create a new movie and use it.

Code Example

Using the previous Movie example, what we want to do is move all code that is about validating a movie's data into a new class.

Let's start with the first test for our new MovieValidator.

RSpec.describe MovieValidator do
  let(:valid_movie) {Movie.new(name: "the name", release_date: DateTime.parse("08 June 1984")}

  it "validates a movie" do
    expect(MovieValidator.valid?(valid_movie)).to be_truthy
  end
end

MovieValidator has a method valid? that returns true or false, depending on whether the given movie is valid or not.

A solution we come up with to make that test pass might look like this in the end:

class MovieValidator
  def self.valid?(movie)
    has_title?(movie)
  end

  private

  def self.has_title?(movie)
    movie.title.present?
  end
end

But what happens when a movie is not valid? We need a way to return validation messages to clients of MovieValidator. Later, these messages could be displayed to an end-user, for example. Let's capture that with new tests that verify these messages.

RSpec.describe MovieValidator do
  let(:valid_movie)   {Movie.new(name: "the name", release_date: DateTime.parse("08 June 1984")}
  let(:invalid_movie) {Movie.new}

  def validation_of(movie)
    MovieValidator.validate(movie)
  end

  it "validates a movie" do
    expect(validation_of(valid_movie)).to be_okay
  end

  it "does not contain any messages" do
    expect(validation_of(valid_movie).messages).to be_empty
  end

  it "does not validate an invalid movie" do
    expect(validation_of(invalid_movie)).not_to be_okay
  end

  it "has messages for invalid movies" do
    expect(validation_of(invalid_movie).messages).to contain("Name cannot be empty.")
  end
end

To make these tests pass, we introduce the concept of a validation result.

class MovieValidator
  def self.validate(movie)
    new(movie).validate
  end

  def initialize(movie)
    @movie = movie
  end

  def validate
    result = Result.new
    result << validate_title
    result
  end

  private

  attr_reader :movie

  def validate_title
    "Title can't be empty." if title_empty?
  end

  def title_empty?
    movie.title.blank?
  end

  class Result
    attr_reader :messages

    def initialize
      @messages = []
    end

    def okay?
      messages.empty?
    end

    def <<(message)
      @messages << message if message
    end
  end
end

Whenever a new requirement for the validity of a movie comes up, we can implement it isolated from all other uses of Movie centralised in MovieValidator. The final version of the test and the production code are available on GitHub.

Instead of rolling our own validations, there's also the option of including ActiveModel::Validations. This is a viable alternative depending on the use case at hand, as we can make use of the already existing implementation almost without change.

class MovieValidator
  include ActiveModel::Validations

  validates_presence_of :title
  validate              :release_year_not_before_1895

  def initialize(movie)
    @title        = movie.title
    @release_date = movie.release_date
  end

  private

  attr_reader :title, :release_date

  def release_year_not_before_1895
    if release_date.nil?
      errors.add(:release_date, "must be provided.")
    elsif release_date.year < 1895
      errors.add(:release_date, "is older than the world's first motion picture.")
    end
  end
end

After extracting the validations, how does the Movie class look after these changes?

class Movie < ActiveRecord::Base
end

There's some truth to the best code is no code at all. We managed to get rid of all custom logic in our model. This leaves us with Movie only being our gateway to persistency, and nothing else.

Which tests for Active Record do we keep then?

Ideally none. But let me explain that.

No tests at all goes a bit too far. In the end we still need to make sure that a Movie gets persisted at some point. But we shouldn't verify that Active Record works as expected. That's a dependency we expect to function properly.

What we do want to verify, though, is that the part of our code that interacts with Active Record does the right thing. This is currently specified in the tests and implementation of MovieController.

In an upcoming part of this series we will go into more detail of whether that's the right place for it, or if there are better alternatives.

Christoph Gockel, Software Crafter

Christoph Gockel is a Software Crafter who enjoys writing clean code almost as much as watching Ghostbusters.

Interested in 8th Light's services? Let's talk.

Learn more about our Ruby services

Contact Us