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

@dhh I have this feeling that you are aware of the fact that this shown refactored version is highly exaggerated, and not what TDD would strive for.

Jim said himself that he normally would not apply the pattern to this example because it is so simple. He specifically says that we should imagine that there is more stuff going on.

So why is this example picked again? To show something that Jim says himself? That this is a bad example?

I'm really disappointed that once again things are conveniently ignored to show of how TDD is bad.

Please watch the video. Jim specifically says that this is an oversimplified example and that you should not apply any decoupling patterns in this situation. He is only doing it to show how it is done.

@stravid Well said.

This evented/callback style seems better suited for a desktop or mobile application. In those cases, there is a desire to keep the UI responsive to user input and I think decoupling with events (or "reactive" or "KVO" or whatever you are calling it) makes sense. In the context of a web application, where the main user interaction is an HTTP request, I think this amount of decoupling is confusing.

I reached the same conclusions as others after watching Jim's video: it was an interesting experiment, but I wasn't convinced that I would like working in such a system.

Maybe a more middle-of-the-road approach would be something like Command-Query Separation (CQS). The controllers would be simply delegating to Command or Query objects, which could be tested in isolation (if desired) - but still maintaining the procedural, top-to-bottom flow of execution. You could probably argue that AR models can play both the Command role (#save, #update) and Query role (#first, #where) without the need for additional abstraction.

The problem with these sorts of simple examples is their simplicity. Any task simple enough to be discussed within a time limit that most of us would be willing to spend reading an article or watching a video makes the process being illustrated appear overwrought.

This, I think, is one reason why we have people who speak from hard-won experience about the benefits a larger inventory of objects with smaller, easier to understand, intention-revealing APIs can bring, but very few compelling examples in the form of blog posts, talks, and the like. The domain complexity these patterns help us cope with would necessitate that more time be spent explaining the domain than the approach being taken, itself.

Are there canonical examples of more complex workflows that are readily understood and provide a jumping off point to work from? I'm exhausted with todo apps, blogs, project managers, and the like. I really liked @ntl's example of promoting an employee. Promotions, payroll, etc, seem like a good start.

The irony is that the lack of attention span that makes our eyes glaze over when we read swaths of procedural code looking for the one thing we actually care about is the same thing that prevents us from tolerating long-form exposition of approaches to help us deal with the problem.

Jim Weirich actually weighed in on this in his wyriki repository: https://github.com/jimweirich/wyriki/blob/master/app/controllers/pages_controller.rb

Below is a snippet of code from his project.

class PagesController < ApplicationController
  include PageRunners

  def show
    @wiki, @page = run(Show, params[:wiki_id], params[:id])
  end

  def show_named
    run(ShowNamed, named_params[:wiki], named_params[:page]) do |on|
      on.success { |wiki, page|
        @wiki = wiki
        @page = page
        render :show
      }
      on.page_not_found { |wiki_name|
        redirect_to new_named_page_path(wiki_name, named_params[:page])
      }
    end
  end
end

My understanding is that he was presenting guidance for when the design had grown beyond the simple case.

I feel this is extremely unfair of @dhh to use this example because the author is no longer able to respond. As @stravid said Jim did not advocate this style for simple scenarios.

Jim said a lot of things in that talk. Among them:

• He advocated an incremental approach.
• He specifically said he wouldn't use this approach for simple CRUD operations.

He used a simple use case so the architectural components would be more easily identifiable. Also, his primary goal was domain isolation from external dependencies. Fast tests were simply a by-product of that goal.

@dhh is there an example you could use from someone who could respond directly? This would enable a direct conversation rather then this weird side ways situation. Unfortunately Jim can no longer speak for himself.

Additionally Jim's https://github.com/jimweirich/wyriki repository has a more refined and evolved example with some significant improvements over the material from his Cincy Ruby talk.

I think this blog post from Uncle Bob is also relevant to this discussion. http://blog.8thlight.com/uncle-bob/2014/05/01/Design-Damage.html

Jim made it clear, in his video, that he would not begin an application this way. Indeed, the whole point of the video was to take an application that had begun the traditional rails way, and refactor it towards an hexagonal architecture.

He also explained when and why you'd do this. His quote, at the end of the video was:

"The thing I want to stress is that: I don't think Rails is evil. I don't think it's a bad framework. I think that as applications grow what it gives you by default is not good for growth."

As applications grow, the coupling to "convenient" frameworks becomes less and less "convenient" and more and more costly. There comes a crossover point where continuing with the old pattern is greater than the cost of refactoring. In my experience that crossover point is fairly early on in the life of an application. YMMV.

I've been struggling with understanding how to apply the hexagonal architecture to Rails since I saw @unclebob's presentation at Midwest RubyConf in 2011. I don't think this is quite the right direction; I'm still searching.

This refactoring doesn't make much sense in any case. (Except maybe the repository pattern, but that's not the gist of this gist.) Getting rid of the direct reference to Employee doesn't really gain us much. We still have to worry about the details of the employee object in the redirect_to and render calls. So we still have a dependency in that direction, but we've added a dependency in the opposite direction with the callbacks.

But this doesn't have anything to do with TDD. We can (and my teams have) easily test the controller as originally without having to instantiate real Employee objects or hit the database. And although I'm a TDD proponent, I personally don't like to test Rails controllers. I write them as such a thin layer of boilerplate that I have enough confidence that they'll work just fine, and have user acceptance tests (with Capybara) to back me up in case I'm wrong.

So I'm afraid this is yet another strawman against TDD. Especially since TDD doesn't necessarily lead to the hexagonal architecture, and the hexagonal architecture is about decoupling, not testability.

"Maybe I need to set a variable for my views; maybe I want to filter on params; maybe this action's models need to be scoped to an association; and before you know it you've spent a lot of time dealing with the meta magic inherited resources provides and it's no longer clear, in my opinion, at a glance what is doing what and where."

This is YOU fighting RAILS, stop it. Let Rails have it's opinions here.

"I feel this is extremely unfair of @dhh to use this example because the author is no longer able to respond. As @stravid said Jim did not advocate this style for simple scenarios."

I do believe it was Gary that started it, and it wasnt clear to him this was someone cargo culting Jim doing "playtime for the mind" as he called it (at least in my session).

I didnt read the rest as you apparently didnt read the first :)

EVERYONE STOP!

No one (with any amount of experience in Rails dev) is saying you should do this, or that this is hexagonal. Hexagonal is bout YOUR DOMAIN.

So you have an app that is about a local car marketplace. Now do this mental exercise:

Make space in your mind for 2 lists.

In the first, put everything that the FRAMEWORK deals with... hint.... HTTP, sessions, persistence, validation, html, css, etc

Now make a second list, that list is simply EVERYTHING that isn't in the FIRST list that has to do directly with your DOMAIN.

Now your DOMAIN MODEL will benefit from hexagonal architecture as driven by TDD. Also, any SERVICE OBJECTS you can extract out of Rails can as well.

But if you are talking about MVC, and you try to change MVC and not the domain. You are doing Test Damaged Design.

This went from a straw man, to a burning straw man, with hexagonal being burned at the stake and Jim being misrepresented.

TDD IS AWESOME, HEXAGONAL DESIGN IS AWESOME

Mutilating Rails is not. If you want some shit like the above, do what DHH did, do what Gary did and write a framework where that was meant to happen. :D

if you start a comment like that you gotta finish it with "hammer time" is all I'm trying to say

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' ."

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.