Ruby on Rails 3 refactoring

Question

I have asked that question but there is no response so i'll try to clean it up and ask again.

I have an rails 3 application that holds information about different companies and each company has information associate with it, term, territory, price and etc. The search I would like to do is to find the price for each company based on the length of term, territory and so on. All that is selected from a form. All is working fine when I do it in the view, like so:

<%
for company in Company.all

@territory_group = Territory.territory_group_id(params[:territory], company.id)
@term_type_id = TermType.find(params[:term_type])
@term = @term_type_id.terms.find_term(company.id, params[:term_id])
@prices = @territory_group.prices.c_prices(company.id, @term.id, params[:else])

%>

The result is:

    <% for price in @prices %>
     <%= price.company.name %>
     <%= price.program.name %>
     <%= price.price %>
    <% end %>

#END company loop
<% end %>

Models - the associations between the models are made like this because there are few other models associat with them and cobining is not possible

#Territory model - Each company has different territories organized in groups.
#c(company_id) just finds the company by id
has_and_belongs_to_many :territory_groups

def self.territory_group(territory, company_id)
 z = find(territory)
 z.territory_groups.c(company_id).first
end

#TerritoryGroup model
has_and_belongs_to_many :territories
has_and_belongs_to_many :prices
belongs_to :company

#Term model hold periods of time - term_from, term_to and term as fixed date
has_and_belongs_to_many :companies
has_and_belongs_to_many :term_types
# c & t are integers / c - company, t - term
scope :find_term, lambda {|c,t| where("company_id = ? AND term_from <= ? AND term_to >= ? OR term = ? AND company_id = ? ", c,t,t,t,c)}

#TermType model – the ralation here is that term can have different term types depending on the company
has_and_belongs_to_many :terms

#Price model belongs to few different models and accepts different params 
#scope :c_price – finds the price for each company, term and some other parameters 
  belongs_to :company
  belongs_to :term
  has_and_belongs_to_many :territory_groups

My question is how to refactor all this, so the only thing left in the view is showing the result of the search, so later on I can export the results in xml and json?

Thx in advance!

Zachary Allaun · Accepted Answer · 2011-08-23 17:36:09Z

For starters, the following code:

@territory_group = Territory.territory_group_id(params[:territory], company.id)
@term_type_id = TermType.find(params[:term_type])
@term = @term_type_id.terms.find_term(company.id, params[:term_id])
@prices = @territory_group.prices.c_prices(company.id, @term.id, params[:else])

could be abstracted into your controller for the appropriate action (which I will assume to be Index). When your Index action is called, then, @prices (as well as @territory_group, @term_type_id, and @term) will be available in the corresponding view, and your for loop should still work. This should be true for all instance variables declared in your controllers; that is, they should be available in the corresponding views for the actions in which they're declared.

Example companies_controller.rb:

class CompaniesController << ApplicationController
...
  def index
    @territory_group = Territory.territory_group_id(params[:territory], company.id)
    @term_type_id = TermType.find(params[:term_type])
    @term = @term_type_id.terms.find_term(company.id, params[:term_id])
    @prices = @territory_group.prices.c_prices(company.id, @term.id, params[:else])

    respond_to do |format|
      format.html
      format.xml { render :xml => @prices }
    end
  end
...
end

In general, you want to keep your logic in a controller, as opposed to a view.

Is this what you were looking for? Or am I missing something else?

EDIT Could you capture the results from your company loop in an array, and then run through them using the each method? Like so:

class CompaniesController << ApplicationController
...
  def index
    @prices = []

    for company in Company.all
      @territory_group = Territory.territory_group_id(params[:territory], company.id)
      @term_type_id = TermType.find(params[:term_type])
      @term = @term_type_id.terms.find_term(company.id, params[:term_id])

      @prices << @territory_group.prices.c_prices(company.id, @term.id, params[:else])
    end

    render_to do |format|
      format.html
      format.xml { render :xml => @prices }
    end
  end
...
end



[VIEW:]

<% @prices.each do |price_set| %>
  <% for price in price_set %>
    <%= price.company.name %>
    <%= price.program.name %>
    <%= price.price %>
  <% end %>
<% end %>

For each company there is different price, so running the frist loop I get the company and trough active record associations with term, territory and etc, I get to the price I want and then with new loop I finaly get the price I need for each of the companies. I see the queries in the terminal and all is seems good, but when go to the view @prices is empty.
I actually tried to do it this way and got fixnum error in prices << teritorry_group.prices. Anyway thank you for the help, it's much appreciated.
No it did work :-) I was saying that before I posted here I tried the exact same way and got an error, but now is working just fine. Thank you!
What if I want to do some additional stuff in for price in price_set loop and abstract the loop in the controller as well. For example the loop is in the controller and I alter the price if the company has discount or increase depending on some variables unique for the company. How do I get the result out to the view then?
First, I'll admit something: I'm learning Rails! With that said, there very well may be a simple and straight-forward answer to your question, but I don't know it. It seems to be that abstracting the price_set loop to the controller would be difficult. The prices are kind of "locked up" inside the loop, so I'm not sure how to add discounts, etc, at the moment. I'll keep thinking on it, though!

Collectives™ on Stack Overflow

Ruby on Rails 3 refactoring

1 Answer 1

6 Comments

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

6 Comments

Related