MVC design question

How much business logic is actually allowed in a (Rails) controller? In the attachement I've put down three different approaches to the same problem. Which if the three approaches is preferable?

1) Calling Payment.create! directly. 2) Calling Payment.create! from Order#add_payment wrapper. 3) Calling Payment.create! from Order#add_payment wrapper without checking Order#payable from controller.

Am I violating any design patters with the last, and is it wise to wrap around the Payment.create instance method in Order#add_payment?

controller_model_approaches.rb (1.29 KB)

Since this is a philosophical question you are bound to get many different answers, no one answer will be "it" or correct for all situations.

Item 1 is clear to the reader what is happening but requires the controller to know about how the models relate to each other. Item 2 has the advantage that how order and payment are related is not known by the controller, just that they are related.

I think that the add_payment method should do any checking since its value is to hide the implementation of the connection between the two classes from the controller. If the controller knows what relation to test it serves no purpose to hide that in the add_payment method.

Michael

Thanks for these very helpful replies, I’ve chosen to call Order#add_payment directly from the PaymentController without any conditions, besides checking the parameters, which is a controller thing. Order#add_payment calls Payment.create! on certain conditions. And raises an exception if those conditions aren’t met e.g. Order#payable? returns false. Since there are several conditions on which Payment.create! should not be called, Order#add_payment raises specific exceptions for each of those conditions, this way I can provide error messages to the users without having a bunch of conditions in PaymentController#create which /must/ be called in order to keep the business logic alive.