Rails: Skinny Controller Vs. Fat Model, or Should I Make My Controller Anorexic

Rails: Skinny Controller vs. Fat Model, or should I make my Controller Anorexic?

You should give Trailblazer a go. This is a thin framework on top of Rails (actually, it runs with all Ruby frameworks), it introduces a service layer, form objects, indirection between persistence and domain code, and so on.

It really helps to keep all your software components skinny as it gives you more abstraction layers that make it so much easier to figure out where to put what.

For example, why would you press your validation logic into controller and model when you have a form object for that?

How skinny should controllers, and how fat should models be?

Controllers should be directing traffic. Models are for where your business logic goes, so in general your second example would be "correct mvc".

Basically what a controller should be doing is requesting input, telling models to change state (they do the actual state change themselves), and determining which view to display (if any).

I have many controllers which simply are like so:

class Controller_Foobar extends Controller
{
    public function action_index() {}
}

And if they need to process $_POST input, they grab that data, and send it off to the model, then the view.

Keeping all that logic inside your models lets you easily reuse it, and it's more maintainable and testable.

Deciding whether def should belong to controller or model (Ruby)

I've worked in many environments, in many languages, ranging from entirely procedural, to object-oriented but not MVC, to MVC with fat controllers and MVC with skinny controllers. I can only speak of my own opinions, but these are things I've learned over the years and opinions I've gained through experience and having to deal with the maintenance consequences of some of the early code I wrote (we all have a past!).

I also know that many people will disagree with what I write here, as that is the nature of how we work ;)

1. Fat controllers are generally bad. It probably indicates that you have model logic in your controllers and where models are used in more than one controller, there's a good chance some of that logic is being needlessly duplicated.
2. Controllers should give as little information as needed to their views, and the views should be able to "pull in" what else they need, by asking one or two core models for it (e.g. don't worry about passing in a model and its associations as separate variables. Pass in the model and let the view fetch the associations if needed. I've worked on systems (in fact, I'd say, unfortunately, most systems) that pass 10's, 20's, 30's of variables into a view, when many of those could simply have to been loaded on-demand by the view itself. I think Rails generally does a good job of encouraging a model-pull methodology, but I'm talking about MVC (on the web) as a broader concept.
3. Views can use complex logic. Some projects I've worked on thought that because something in a view was more than a simple "for each" loop, it therefore needed to be stuffed into the controller. That is wrong. Your controller is then performing view logic. The view is code... let's not hide that fact.
4. Relating to point (3), don't confuse "template" with "view" as a whole. The template is just part of your view.

I'm drifting off-topic here, but in short, most of your logic should probably finish up in your models. Your models, well, model your application in terms of data and how that data changes. It's therefore natural that this is where a lot of logic is placed. Your controllers only serve to transport information between the models and your end user (which just so happens, to be via the view).

Quite a long-winded way to say "I agree with John", eh? ;)

Move from the (soon-to-be-skinny)controller to the (not-so-fat)model

You can simplify your controller by creating a scope (as @apneadiving demonstrates), or by a method that encapsulates the query:

def self.amount_sum(user)
  where(:user_id => user).sum(:amount)
end

then in your controller:

@total_of_items = SomeModel.amount_sum(@user)

In this case I would use the method, because the scope is a little harder to read. Ryan Bates makes note of this in his Railscast 215:

In the second named scope we’re using
a lambda. If you ever use one of these
in a named scope you might consider
using a class method instead
especially if you’re passing in a
large number of parameters or if the
content of the scope is complex. Ours
is fairly simple but we’ll turn it
into a class method anyway.

Your case is pretty simple, but I'd consider placing the logic inside a class method.

What kind of logic should be in Rails before filter

Yes, finding a record and setting it as an instance variable is a common convention for controller filters. Generally though, any piece of code that gets run for multiple actions is a good candidate. Say you want to redirect to the log in page if the current user is not logged in.

class UsersController < ApplicationController
  before_action :require_login
  before_action :set_user, only: [:show, :edit, :update, :destroy]

  private

  def require_login
    unless logged_in?
      flash[:error] = "You must be logged in to access this section"
      redirect_to new_login_url # halts request cycle
    end
  end

  def set_user
    @user = User.find(params[:id])
  end
end

Rails: how to handle after_create: create_subscription?

Although generally "fat model, skinny controller" is a good rule of thumb, it's really just there for inexperienced developers who don't push enough down to the model.

The real rule should be "the code should sit where the code belongs". If the subscription is dependent on user input then it doesn't necessarily belong in the Topic model.

Sometimes it makes sense to package processes into a service object that your controller can call. It will accept your input, do some actions including updating one or more models as appropriate, and return a result.

https://netguru.co/blog/service-objects-in-rails-will-help

Where to put create model and association logic in Rails?

Your example does not seem very complex and can be done using nested attributes as mentioned in the comment. For more complex stuff, there is nothing provided by the framework and you have to come up with your own abstraction.

Have a look at service objects:

1. https://www.engineyard.com/blog/keeping-your-rails-controllers-dry-with-services
2. https://medium.com/selleo/essential-rubyonrails-patterns-part-1-service-objects-1af9f9573ca1

Again since these are community abstractions, there is no standard way of implementing service objects - some people use classes with call method, some use modules, etc. How you use them depends on your definition of clean code.

---

Related Topics