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

Question

I know similar questions have been answered before - such as: Where should logic go, where to do certain tasks, etc. But I have a more specific question - How far should I take this axiom: "keep your controller skinny, make your model fat!"

Here is an example:

For instance let's say I have multiple source of verification data. A good example would be a VIN number - I can verify it against, manufacturers data source, DMV's data source, also my local databases - to see what I have on record. So I have a model called Vin and vins_controller. Inside the model I have 5 methods: check_against_local_db, check_against_dmv, check_against_car_maker_1, check_against_car_maker_2, etc.

In my controller keeping with the REST, in action show - I have a simple case statement which looks at the params[:source], and based on source specified - will call specific check method.

Now here is the question: Should I leave the logic that governs which data source to call in controller or should I move it to model and then in controller just do something like check_vin(source, vin)?

Should I make my controller anorexic?

Answer 1

I would put the logic in my model, especially if I'm TDD'ing (and I always TDD, except when I don't.) Testing the model is usually much easier than testing the controller.

Answer 2

I like to approach questions like this by thinking about responsibility. What in this case is "responsible" for verifying the VIN? The model. The controller is simply there to pass along the parameters...to "control" based on user input.

If it's not entirely clear, think of it this way: where will putting this code cause the least amount of impact if it needs to be re-used? Say...if two different actions in two different controllers both need to verify a VIN, what would need to be done? If you left this code in the controller, you'd essentially have to duplicate it in the new controller as well, but if you had placed it in the model, you'd simply call the check_vin method from the new controller, and no code duplication would be needed. By assigning responsibilities where they make the most sense, you've improved the re-usability of your code.

Answer 3

There is an old saying,

Smart data structures and dumb code works a lot better than the other way around.

Everything you have described is about how one piece of data relates to another. That itself is your clue that the logic for those relationships, including validation should be in the model or database layer.

An anorexic controller is a sign of well-designed (smart?) data. My experience tells me that the more effort you put into designing your data, the less code you have to write overall.

Controllers are best at parsing inputs, calling the appropriate models, and then formatting the outputs.

Answer 4

It's the controller's responsibility to "parse" params, but validation should be done in the model.

I would do something like this on the controller:

@my_model = MyModel.new(:source => params[:source] ...)
if(@my_model.valid?)
  # treat valid model here (i.e. save the model and redirect to show)
else
  # treat validation error here (usually throw an error)
end

Your controller will actually be just "skinny". For truly "anorexic" controllers, you might want to have a look at inherited_resources or resource_this. In some cases these will give you a 3-lines controller, implementing the whole RESTful stack.