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

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.