CODEWITHSUNDEEP
ruby-on-railsruby
execv
execv

Reputation: 849

Rails: Cutting Down Code

I know that I might have too much logic in my view, so I'm wondering how I can include it in my controller:

Controller:

def purchasers
   @deal = Deal.find(params[:id])
   @pl = @deal.purchases
end

View:

<% title "List Of Purchases" %>
Total Purchases: <%= @pl.count %><BR><BR>
<%
@pl.each do |p|
    u = User.find(p.user_id)
    %>
    <%= u.email %><BR>
 <%
end
%>

Upvotes: 0

Views: 73

Answers (4)

Pete
Pete

Reputation: 1482

Its also worth noting the following can be improved to make use of Rails' skills when it comoes to caching collections:

<%= @pl.count %>

to

<%= @pl.size %>

The size method will return the number of purchases but won't load the objects into memory again as they have already been looked up in the controller.

Upvotes: 0

Mehmet Davut
Mehmet Davut

Reputation: 667

if your Purchase model belongs to User model, you don't need to find User with User.find. if not, belong your Purchase model to User model then

<% @pl.each do |p| %>
  <%= p.user.email %>
<% end %>

Upvotes: 0

justinbach
justinbach

Reputation: 1955

It looks like you might not have set up your associations correctly in your Purchases and Users models. Instead of doing u = User.find(p.user_id) you should be able to write p.user.email, assuming that each Purchase belongs_to :user.

Upvotes: 0

chrismdp
chrismdp

Reputation: 58

I'd suggest that you remove the call to User.find inside the view code.

It looks like you're looking up the user from the user_id stored in the purchase. Why not in the model use:

class Purchase < ActiveRecord::Base
   belongs_to :user
   ...
end

And then in the view code:

<% @pl.each do |purchase| %>
  <%= purchase.user.email %><BR>
<% end %>

Hope this helps.

Upvotes: 4

Related Questions