tags:

views:

46

answers:

3

Would it be ok to put an "average price" calculation, as below, in the view?

Or is this against MVC and is it better to do this in the controller?

<p>Average price: <%= @seller.total_sales / @seller.num_sales %></p>
+4  A: 

Neither. Put it in the model. Then it becomes easy to unit test.

John Topley
It's easy enough to test in the controller, too, but clearly it ought to be a property of the model otherwise you'd have to calculate it in each controller action where it was needed.
tvanfosson
I'm not familiar with unit testing, sorry. What would be the importance of unit testing this?
eggdrop
Indeed. Business logic belongs in the model.
John Topley
"clearly it ought to be a property of the model otherwise you'd have to calculate it in each controller action where it was needed" - yes, this makes sense. thanks.
eggdrop
You would unit test it to ensure that you're calculating the average correctly.
John Topley
@eggdrop - to make sure it's calculated correctly, of course. It may seem overkill in this case, but consider what happens if you change total_sales to be the amount before mark up and add a new property to track sales after mark up. With unit testing (and putting the property in the model), you'd catch right away that your average price calculation is no longer correct.
tvanfosson
@tvanfosson - But, down the road, if I change total_sales to be the amount before mark up, then my unit tests may end up being incorrect also - if they rely on total_sales for calculating the correct average? Don't I just shift the location of the problem? (sorry if I'm not understanding unit tests - I've never actually written any so I'm not sure what they are exactly).
eggdrop
A: 

Ask you several things:

Will this average price will be often displayed Is it a part of a view (is it used to display something ?) Does it need complex things to get/calcul/retrieve or whatever ?

If you think it's just an hint for your user, it's used only once, then you can let it in your view.

But if you feel unconfortable with it, or you need to do more complex maths on the price, put it your model..

Boris Guéry
"If you think it's just an hint for your user, it's used only once, then you can let it in your view." - What if the MVC police catch me?
eggdrop
A: 

Put your business logic where it belongs in the model:

<p>Average price: <%= @seller.get_average_price () %></p>
Todd Smith