views:

97

answers:

5

i have Product and SalesOrder model (to simplify, 1 sales_order only for 1 product)

Product
 has_many :sales_orders

SalesOrder
 belongs_to :product

pa = Product A #2000
so1 = SalesOrder #1 order product A #1000, date:yesterday
so2 = SalesOrder #2 order product A #999,  date:yesterday
so3 = SalesOrder #3 order product A #1000, date:now

Based on the date, pa.find_sales_orders_that_can_be_delivered will give:

SalesOrder #1 order product A #1000, date:yesterday
SalesOrder #2 order product A #999,  date:yesterday
SalesOrder #3 order product A #1,    date:now <-- the newest

The question is: is find_sales_orders_that_can_be_delivered should be in the Model? i can do it in controller.

and the general question is: what goes in Model and what goes in Controller.

Thank you

+3  A: 

It's simple :

Tiny Controller, Fat Model

You made all in your model. If you put in your controller you can't reuse it in different part.

  • Rake task
  • Other controller
  • Console

So put all possible in your model. In Controller you put only manipulation with session/cookie. The redirect part and which view is use.

shingara
i see that. Thank you.
Hadi
+2  A: 

I think this logic should go to Model as this is related to objects' properties.

For solution to generalized problem- you may like to read it up here : http://weblog.jamisbuck.org/2006/10/18/skinny-controller-fat-model

Nikhil Garg
thanks for the link. It helps me understand much more.
Hadi
+1  A: 

Here's a nice article on the MVC pattern in Rails. To sum up: controllers should be lean, and models fat ;-)

JRL
A: 

I think this should be in the model. Generally when I want to decide whether something goes in the controller or the model I ask myself "Do I want this if I access the data in another way?".

For example, if the data is being accessed by another controller, like an admin tool, do I still want to use that functionality? Would I still need it? Then it is related to the model, not the controller.

Another example: if the data is being accessed by another app, a desktop app for example, would I still want this functionality? Same as before.

J. Pablo Fernández
A: 

Absolutely, keep this logic out of the controller. Controllers should hand off business decisions to the models. This is the entire point of having an MVC framework to begin with.

Additionally, I'd put the logic on SalesOrder, not Product -- it looks like a good candidate for a named_scope. Maybe something like this (and keeping in mind I have no idea what your specific rule is for calculating the date of of a deliverable order):

named_scope :deliverable, lambda {{ :conditions => ["date < ?", Date.today] }}

Wrapping with a lambda makes sure the :conditions are evaluated in real time, using the current date value. Obviously the :conditions are dependent on whatever your business rule is for deliverable orders, but you get the idea. This would allow a nice method chain like:

pa.sales_orders.deliverable

Now you can re-use that logic wherever there's a has_many :sales_orders.

Dave Sims
named_scope looks nice, but i doubt that it is doable, because deliverable require to look in the whole database, compare the product stock (and virtually reducing its value) as one sales order is found that has the required items. Well, to make it simple, it require assigning variables in sales order how many of such product can be delivered. I hope you get what i mean.
Hadi