You're a Rails programmer. You've developed your site and have a good set of features that you're delivering to your users. You use RSpec to test your application and have Cucumbers pushing WebDriver tests. You may even be doing some cool process things like continuous deployment. But, do you write Ruby or do you write Rails?

Writing Rails

Rails is a great web framework that has helped many teams deliver business value in a very short amount of time. It helped to create the mantra of Opinionated Software that exists in the Ruby community and has even started to trickle down to users (for the benefit of all, in my humble opinion). Rails has also been a great leader in the area of testing and craftsmanship in the greater software practice.

That being said, Rails has become a crutch for many people (as all frameworks tend to do). It encourages a very flat world of MVC where the models can go get themselves out of a persistent store and controllers and views can access everything.

How much code do you have that is not MVC?

Name the number of classes in your code that do not inherit (directly or indirectly) from ActiveRecord::Base (or, in my case, Mongoid::Document), ActionView::Base, or ActiveController::Base. Is it more than zero? Is it a significant percentage of your application? Is it most of your application?

At a certain point your web application will become more than a CRUD web server where users manipulate objects in a database. Let us take for example GitHub. When you first log in to GitHub, you are sent to your dashboard. I'm going to write some bad code:

class DashboardController < ApplicationController
  def index
    # we're using devise and the user is logged in (checked in ApplicationController)
  end
end
.dashboard
  .news
    - current_user.alerts.limit(20).each do |alert|
      .alert
        .type= alert.type
        .header= alert.commit? ? commit_header(alert) : pull_request_header(alert)
        .message
          .committer_image= alert.commit.committer.gravatar
          .commit_mesasge= alert.commit.message

Excuse the incompleteness of the example but this is enough to illustrate. This is a Bad Thing™ because we are doing many N+1 queries against our SQL database. Easy enough to fix. We should pre-fetch all of our dependencies. I am going to use a more explicit way instead of trying to let the ORM do my work for me, again for illustration.

class DashboardController < ApplicationController
  def index
    @alerts = current_user.alerts.limit(20)
    @commits = Commit.where(:_id.in => @alerts.map(&:commit_id)).to_a # using Mongoid here
    @committers = User.where(:_id.in => @commits.map(&:committer_id)).to_a # using Mongoid here
  end
end
.dashboard
  .news
    - @alerts.each do |alert|
      .alert
        .type= alert.type
        .header= alert.commit? ? commit_header(alert) : pull_request_header(alert)
        .message
          .committer_image= alert.commit.committer.gravatar
          .commit_mesasge= alert.commit.message

Alright, so now we've taken care of the N+1 query issue. However, I'm loading a ton of unnecessary data from the database, building it into objects in member and then garbage collecting it later down the road. We're GitHub and we cannot afford this luxury, so we cut the fat (always test that this sort of optimization is necessary though!!).

class DashboardController < ApplicationController
  def index
    @alerts = current_user.alerts.only(:type, :commit_id).limit(20)
    @commits = Commit.where(:_id => @alerts.map(&:commit_id)).only(:message, :committer_id).to_a # using Mongoid here
    @committers = User.where(:_id => @commits.map(&:committer_id)).only(:gravatar).to_a # using Mongoid here
  end
end
.dashboard
  .news
    - @alerts.each do |alert|
      .alert
        .type= alert.type
        .header= alert.commit? ? commit_header(alert) : pull_request_header(alert)
        .message
          .committer_image= alert.commit.committer.gravatar
          .commit_mesasge= alert.commit.message

And, we're starting to notice that our .header= HAML line is really big. We could pull it into another helper method. Or, we could pull it up into the model itself. Which is it? View logic or model logic? For sake of my example, let's put it in the model.

class Alert
  include Mongoid::Document
  
  def header
    commit? ? commit_header : pull_request_header
  end
end

And we notice that our controller method is getting big-ish. We should move that code to the model, as well, right?

class Alert
  def self.alerts_for_dashboard(current_user)
     alerts = current_user.alerts.only(:type, :commit_id).limit(20)
    commits = Commit.where(:_id => @alerts.map(&:commit_id)).only(:message, :committer_id).to_a # using Mongoid here
    committers = User.where(:_id => @commits.map(&:committer_id)).only(:gravatar).to_a # using Mongoid here
    alerts
  end
end

And this is the state in which we push the code to production and call it a day.

What about Ruby?

We forgot about using Ruby itself at some point in learning how to develop Rails applications. I would contend that the model is the place for neither of these peices of code. The header code is not part of the model but is instead presentation logic.

On helpers: helpers are the default way to encapsulate presentation logic in Rails. There is nothing wrong with helpers themselves but they start to get out of hand when you start to pull in many different objects, cascade helper calls and use helpers from many different helper files. When that starts to happen, we need to move away from helpers to a more robust method of handling presentation logic.

This dashboard view should probably have something called an AlertPresenter (for another pattern, look at Avdi's Exhibit Pattern). The AlertPresenter would look something like this

class AlertPresenter # look ma, I'm an Object
  def initialize(alert) # look dad, I'm using explicit constructor arguments!
    @alert = alert
  end

  def header
    @alert.commit? ? commit_header : pull_request_header
  end
end

This would encapsulate all of the logic needed to write the "header" of the GitHub dashboard alert in one place. If I ever needed to change what that looks like, I know exactly where to look. Another side benefit of this class is that I do not need to load Rails to run this test. That means the test can run fast. Very fast.

The other peice of code is not presentation logic, but persistance logic. Because of ActiveRecord, we have started to conflate persistance and domain logic in our codebases. This is a bad trend. It creates confusion and a dangerous amount of coupling in our code. While I could go on about the benefits of a DataMapper pattern over the ActiveRecord pattern, I will just show you what we could have done with our code to not bind ourselves so tightly with AR.

Instead of defining a class method on Alert, we could create an AlertService that accesses these alerts for us. At this point, it really is just putting the code into another file but it does have benefits. First, its not Rails and doesn't actually require Rails to test. It makes the finding of alerts for the dashboard logic all easy to find and located in a different location. And, it provides a way for us to change how we access it. If, for example, we were to implement CQRS in our application, the Service would be the place to do it. Or, if we decide that we need to implement an HTTP service on the backend that serves us the objects instead of direct access to the database, the service layer can be a point of abstraction for persistance logic. There are more benefits if we take the service/repository pattern further.

class AlertService
  def self.alerts_for_dashboard(current_user)
    alerts = current_user.alerts.only(:type, :commit_id).limit(20)
    commits = Commit.where(:_id => @alerts.map(&:commit_id)).only(:message, :committer_id).to_a # using Mongoid here
    committers = User.where(:_id => @commits.map(&:committer_id)).only(:gravatar).to_a # using Mongoid here
    alerts
  end
end

So, in this very small example, we have decided to create a couple of Ruby objects that make our application more flexible, testable and cleaner.

Cleanliness

I challenge Rails developers to develop a sense of cleanliness of their code. We have a huge commitment to tests and working software. We have a commitment to opinionated software that relies on convention over configuration and to give back to the open source community. It think us Rails developers can learn to appreciate the cleanliness of code. I would define cleanliness as follows:

  • Thin controllers (the same thing we have been doing for a long time)
  • Logicless views (use Presenters/Exhibits to wrap any presentation logic)
  • Domain models (avoid persistance and presentation logic)
  • Repositories [they can be called Services] (for persistence logic)
  • Services (for manipulation of Domain Models -- puppet masters if you will -- this is another area for domain logic)
  • Fast tests that do not rely on Rails (see Corey Haine's Fast Tests talk)

I've only talked about a couple of the things that make clean Rails code possible (Presenter and [Persistance]Service). I hope to talk about other things in the near future.