A better controller?

better_controller.rb

class WidgetsController

  def index
    IndexedResourceCommand.new(Widget).execute(params, default_responder)
  end

  def create
    CreateResourceCommand.new(Widget).execute(params, default_responder)
  end

  private

  def default_responder
    # could have a different responder for different content-types (e.g. respond_to)
    RenderOrRedirectResponder.new(self) # needs self to get access to controller rendering behaviour
  end

end

class ResourceCommand

  def initialize(scope)
    @scope = scope
  end

end

class IndexResourceCommand

  def execute(params, responder)
    responder.respond_with_collection(@scope.all)
  end

end

class CreateResourceCommand

  def execute(params, responder)
    instance = @scope.new(params)

    if instance
      responder.respond_with_saved_object(instance)
    else
      responder.respond_with_unsaved_object(instance)
    end
  end

end

class RenderOrRedirectResponder

  def respond_with_collection(collection)
    # set up assigns and render the appropriate template
  end

  def respond_with_saved_object(object)
    @controller.redirect_to url_for(object)
  end

  def respond_with_unsaved_object(object)
    # set up assigns and render template with errors
  end

end

This approach is definitely "cleaner", but maybe it's too much? Dunno.

Either way, I'm not sure I like that RenderOrRedirectResponder gets a bunch of unrelated methods to render all the possible actions. Maybe something like:

class Responder < Struct.new(:controller)
end

class RenderObjectResponder < Responder
  def respond(object)
    controller.render object
  end
end

class RenderOrRedirect < Responder
  def respond_when_successful(object)
    controller.redirect_to object
  end

  def respond_when_error(object)
    controller.render :new
  end
end

# etc

And then just pass the proper responder on each action. Thoughts?

I'm not 100% sure what the best way of designing the responders would be at this point; I see your point but I'd also be worried about having a combinatorial explosion of responders as soon as you introduce a responder for a different content type (e.g. an XML or JSON responder). It seems ok that a single responder is able to respond with resources in various forms (e.g. a collection, a single unsaved object with errors, a saved object, an updated object etc.).

As to whether it's too much, I kind of feel that if a controller's only responsibility is to pass in the correct resource scope into a command and execute it with an appropriate responder, they become much simpler to write. You could write a little declarative API that makes it a little bit more elegant but ultimately it reduces the amount of boilerplate code as the generally repeated logic for each action is encapsulated in the command.

I also wonder if it's possible to extend rendering/redirection behaviour to Responders without having to pass in the controller (perhaps by mixing in the appropriate modules?)

Actually, you can probably get away with enforcing that the interface is

Responder#respond_when_successful(object)
Responder#respond_when_error(object)

And handle errors by rendering, for example, a 404 page or something like that.

class ShowResourceCommand < ResourceCommand
  def execute(params, responder)
    instance = @scope.find(params[:id])
    responder.respond_when_successful instance
  rescue ActiveRecord::RecordNotFound
    responder.respond_when_error(params)
  end
end

re: mixing modules, I don't think you can. I mean, you can, but you end up needing that self is an ActionController::Metal, so it's the same thing.

Actually, I'm thinking that most of the Responder stuff can be replaced with respond_with.

class ResourceCommand < Struct.new(:scope, :responder); end

class IndexResourceCommand < ResourceCommand
  def execute(params)
    responder.respond_with scope.all
  end
end

class WidgetsController
  def index
    IndexResourceCommand.new(Widget, self).execute(params)
  end
end

Looks good, I do think this idea has some legs.

One thing I don't really like about the approach is that your creating the dependencies inline within the method. Seems like the roles of the dependencies should be figured out, and then passed into the constructor for the controller object if you were going to go with the command sort of approach. Overall though I agree that a more classic OO implementation would lead to a cleaner controller layer.

Sure, this is really just a dump of the initial ideas in my head. Although, with this approach I'd see little need to unit test the controllers and treat them as boundary objects and cover them integration tests. What would you inject (probably through accessor injection to avoid working around issues with Rails and controller initialisation).

So if I understand you right, your basically using the controller as a port where data comes in? If you were to cover them with integration tests, what would you assert on?

Well, I guess that would depend on the user stories.

I have no idea if this would actually be a good design having not explored it further, more than anything I feel that Rails controllers are probably the weakest part of an app from an OOP point of view and I'm interested in ways of investigating a better approach, whether or not this is it. :)