[ryanbrunner]

There's a difference between "controllers should not contain business logic" and "controllers must be one line". There's plenty of sane responsibilities for controllers that don't directly involve business logic - mapping request parameters to something the business layer understands, coordinating view mapping, applying decorators to models, authentication, authorization, etc.

By rigidly applying an inane rule like "you can only have one line per controller action", you're going to end up just re-implementing controllers, and leaving your controllers doing nothing but pointless indirection.

[tomblomfield]

I agree with this. Statements like "you can only have one line per controller action" are just dogma. What's the point of a controller?

I also don't like ceding control-flow from the controller into the application by injecting the controller context in the form of "rails_adapter", in the name of "Tell, don't ask". It seems like a misunderstanding of the principle.

"As the caller, you should not be making decisions based on the state of the called object that result in you then changing the state of the object. The logic you are implementing is probably the called object’s responsibility, not yours."[1]

Say you have a module that performs some complex business logic: ShipOrder.perform(args). It's responsibility might be to ensure the right items are combined into a package and shipped. It might write some logging data to an injected logging object. It might tell a label-printing to output the right address label. Ultimately, it should succeed or fail, and pass that information back to the caller.

The controller's responsibility, on the other hand, is to direct the control-flow of the request and pass data between models & views (and your other non-model POROs). That's its core responsibility. Passing the controller context into your application object so the application can call-back methods on the controller just seems like a recipe for spaghetti code.

It also means you can't use this object on its own, or composed with other objects, without implementing a complex caller-object that you can pass in to receive the callbacks. If the object just returns success/failure, and exposes a limited API so you can access its internal state if required, it becomes straightforward to re-use & compose the object in places outside the confines of your controller.

1. http://pragprog.com/articles/tell-dont-ask

[sanderjd]

Yeah I agree. I'd be interested to see what that `rails_adapter` implementation looks like. In my experience, this sort of design often just moves a lot of the tricky questions about how to pass data around to the adapter instead of the controller.

Doing rendering based on the application telling the adapter what happened does seem to work pretty well though.

[bestie]

Hi Tom

I have to disagree, by returning a value and switching on it, not only are you querying state but you risk repeating the query code all over the place.

My approach allows you to write a small adapter for your delivery mechanism and use polymorphism to eliminate conditions and duplication.

Most objects cannot be used on their own as they have collaborators. My design allows me to write a trivial console adapter that can puts the results of my service object / interactor.

[nimblegorilla]

I agree, but I see "one line per controller action" as a guideline that doesn't need to be followed religiously. Almost every rails app that I come across has too much domain logic in the controllers. I'm refactoring an ecommerce app where the developers put 400 lines of payment processing code inside one controller.

Things like authorization should be mostly contained in the root controller and not sprinkled around your controller actions.

What kind of decorators are appropriate to apply to models from a controller action? I'm sure there are valid scenarios, but most of that should also go somewhere else.

[ryanbrunner]

If you had a view decorator that applied presentational logic to a model, I don't see a problem at all in including that in a controller. I wouldn't have any issue at all with a controller action that looked like:

    def index
      respond_with PersonListDecorator.wrap(Person.all)
    end

[nimblegorilla]

Yeah I think that is a good idea. But to be fair: your example is only 1 line and has no business logic.