Question

Currently I'm performing some calculations in my views, which is a bad thing, of course:

<% categories.each do |c| %>
  ....
    <%= c.transactions.sum("amount_cents") %>
  ....
<% end %>

I am researching ways that will help me to refactor the above issue.

One thing is to move the calculation to my controller

@category_sum = @transaction.sum("amount_cents")

Which is probably a better solution, but you know. Not perfect.

Since I have many users, I do not see how can I move the calculator logic into my Model. So I guess I might need to use a new Class, create a bunch of methods (sum, average, etc.) and use them in the views? Am I on the right track? Will be thankful for any advice on how to restructure my code and design and implement this Class.

Was it helpful?

Solution

One mean to isolate view logic is to use presenters.

A presenter allows you to do something like that :

<% categories.each do |c| %>
  ....
    <% present c do |category| %>
    <%= category.transaction_sum %>
    <% end %>
  ....
<% end %>

You then have a presenter class in app/presenters/category_presenter.rb :

class CategoryPresenter < BasePresenter
  presents :category

  def transaction_sum
    category.transactions.sum("amount_cents")
  end
end

Of course, it is best used if you have many methods in that presenter (but once you begins to reduce view logic, it's quick to fill presenters).

The implementation used here rely on what is describe in this pro railscast. The basic idea is simply to have a #present helper that infers a class name based on object class, load and initialize the proper presenter class.

An other popular alternative is to use drapper, which use the concept of decorator, but a presenter is basically a decorator.

OTHER TIPS

The main code smell you're seeing is called the law of Demeter (which, like many programming "laws", you should think of it more like a "guideline of Demeter").

What you could do is move the actual calculation step into a method on the category, e.g.

class Category < ActiveRecord::Base
  def transaction_amount
    transactions.sum("amount_cents")
  end
end

<% categories.each do |c| %>
  ....
    <%= c.transaction_amount %>
  ....
<% end %>

Technically speaking the calculation is still performed while rendering the view, but the logic for how that summed amount gets calculated is no longer inside the view itself. All the view now cares about is that it can send the message transaction_amount to the category objects. This also leaves room for you to add a cache for the sums, or for you to stop passing actual records, but instead pass static objects (that aren't ActiveRecord models) that come out of some piece of code that performs the sum in a more efficient manner.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top