This is an extraction from Jim Weirich's "Decoupling from Rails" talk, which explained how to apply the hexagonal design pattern to make every layer of your application easily unit testable (without touching the database etc). It only seeks to extract a single method, the EmployeesController#create method, to illustrate the design damage that's …

test_induced_design_damage.rb

# Original Rails controller and action
class EmployeesController < ApplicationController
  def create
    @employee = Employee.new(employee_params)
    
    if @employee.save
      redirect_to @employee, notice: "Employee #{@employee.name} created"
    else
      render :new
    end
  end
end


# Hexagon-inspired, test-induced, damaged version
class EmployeesController < ApplicationController
  def create
    CreateRunner.new(self, EmployeesRepository.new).run(params[:employee])
  end
  
  def create_succeeded(employee, message)
    redirect_to employee, notice: message
  end
  
  def create_failed(employee)
    @employee = employee
    render :new
  end
end

class CreateRunner
  attr_reader :context, :repo
  
  def initialize(context, repo)
    @context = context
    @repo    = repo
  end
  
  def run(employee_attrs)
    @employee = repo.new_employee(employee_attrs)
    
    if repo.save_employee
      context.create_succeeded(employee, "Employee #{employee.name} created")
    else
      context.create_failed(employee)
    end
  end
end

class EmployeesRepository
  def new_employee(*args)
    Biz::Employee.new(Employee.new(*args))
  end
  
  def save_employee(employee)
    employee.save
  end
end

require 'delegate'

module Biz
  class Employee < SimpleDelegator
    def self.wrap(employees)
      employees.wrap { |e| new(e) }
    end
    
    def class
      __getobj__.class
    end
    
    # Biz logic ... AR is only a data-access object
  end
end

It all depends on what type of application you are writing. If that is the sum total of your controller action, obviously it is over design. Jim was explicitly giving a talk about what to do when you dont have trivial amounts of coordination that needs to happen. The "Rails Way" works great, until the point that it doesn't. At which point, you take the tools Jim was trying to give you, and build something more elaborate to handle the additional complexity.

When you are talking about solving problems of complexity, you are in a difficult position. By definition, you are trying to impart a solution to a problem that is large and nasty, and could easily take a full hour to explain to the audience. So what do you do? Either have a 2 hour talk, or try to show the pattern in a way that can be understood without requiring huge amounts of background knowledge about the domain of the example.

Jim tried to explain this a few times in the talk, I am assuming @dhh must have missed it.

To put it another way, let's say the point of your application is to output "hello world". Rails architecture is MASSIVE overkill for that problem. Does that mean that rails is over engineered? Of course not. It is as complex as it needs to be to solve the type of problems it was built to solve. Presenting that talk in this light is making a very similar sort of claim.

In the video, Jim was exploring ways to decouple his domain code from Rails. It was not polished yet. At the beginning, he says: "What I wanna teach tonight is not 'this is how you do it' but 'here are techniques you can use to do it' ."

Exactly. This is a very important point. Yet most people will only be looking at the code snippet above. Which makes me think it is a bad idea to post the snippet like this if you want a proper discussion. To me this looks like it's set up for misinterpretation. Not cool.

Here's an alternative solution that doesn't disturb DHH's original design: http://patmaddox.com/2014/05/15/poof-and-then-rails-was-gone/

@JeroenDeDauw I agree. This reminds me of the comp.object newsgroup days. After posting how to implement an OO design for some problem using a simple example, there would always be someone who would write the simple example in a single function and try to use it as an argument against OOP. This is the same line of reasoning.

I think that this example

# Original Rails controller and action
class EmployeesController < ApplicationController
  def create
    @employee = Employee.new(employee_params)

    if @employee.save
      redirect_to @employee, notice: "Employee #{@employee.name} created"
    else
      render :new
    end
  end
end

and this example

class CreateRunner
  attr_reader :context, :repo

  def initialize(context)
    @context = context
    @repo    = EmployeesRepository.new
  end

  def run(employee_attrs)
    @employee = repo.new_employee(employee_attrs)

    if repo.save_employee
      context.create_succeeded(employee, "Employee #{employee.name} created")
    else
      context.create_failed(employee)
    end
  end
end

are almost the same, the other classes are just there to integrate this code with rails.

I think I would prefer something like this.

module Employees
  class Actions::RegisterEmployee < Employees::Action
    def call
      employee = Employee.new(params.fetch(:employee))

      if employee.valid_for_registration?
        employee = store.create(:employee, employee)
        responder.success_response(employee)
      else
        responder.error_response(:unprocessable_entity, employee.regstration_errors)
      end
    end
  end
end

If someone shows you a hello world example, it doesn't make sense to say "this example is invalid because I would never want to write a program that says hello world". It's just a very simple example.

Props to @patmaddox there. That really is a beautiful answer. If you haven't looked at it, I encourage you to take a peek. I think it slices through a bit of Gordian knot here, and does it with specs :)

This example is misleading. The talks is about how to modify your design when your application grows.

It's obvious that the original version is better than the modified in this case, but when the code evolves you may want to apply something like if there is a good reason to do it.

Design decisions will be guided by context...

Why don't we look at a controller like this to start with: http://www.slideshare.net/edbond/obie-fernandez-worst-rails-code/35 ?

Like almost every code example, something is missing: context. It's not about 'I might use this in a console application,' it's that 'mysql is not working here, might need a KV store.'

By far I prefer the @patmaddox version. Clean/concise/simple.

To me it looks like this code with no proper context was put here with intention to mislead. To bad Uncle Bob is not on the "Is TDD dead?" talks.

From the perspective of the "Is TDD dead?" debate, I am trying not to get too hung up on this specific code example. As other people have said - a "hello world" scale samples in any approach / framework are never comparable to real world situations. The point for me is, I have seen in many code bases in many companies which have similar characteristics - which are "large scale and reall world". In many cases, the multiple layers and abstractions seem to add no perceivable benefit to the solution. They simply pass data onto other components / layers (which inturn do the same) and so forth. This makes the code harder to maintain (ripple effect across components / layers) and in my experience add to the burden of trying to understand the code. I ask my self, why has this code been added? Is is people thinking they are following "best practice" for isolation purposes? Because it "turned out that way" from following a TDD approach. In my experience, the answer is varied.

For me, the key point is whether the "mockist" approach to TDD, which tends to lead to solutions where all / most behaviour is compartmentalised into individual abstractions (so we can mock them for testing purposes), leads us to better or worse designs.

There is a clear difference here for me between "classic TDD and mockist TDD". The former using state based verification and generally allowing tests with "real" collaborators, compared to the mockist approach, which in my experience tends to lead naturally to more abstractions (and behaviour verification).

In my opinion there is a complexity cost associated with each additional abstraction / layer added to a solution. Whilst the developer "owns the decision" about when to add new abstractions, it is clear in my mind at least, that TDD (in particular mockist TDD) does lead you (though not force you) into creating more. As developers I guess we need to take more responsibility as to when we add new abstractions / layers - and understand the tradeoffs associated. Is abstraction good? Well yes... but that does not mean you apply it verbatim to every single concept... I try to add new abstractions where I can see the benefit - and perform my "tests" slightly more "coarse grained"...

Let's all be civil.

Let's all be civil.