Hi,
I need a little help cleaning up the method below. In my .NET days I
would have let this pass, but I am trying to be a good ruby citizen by
keeping things simple.
This is from a seller model which has many vehicles:
def total_retail_price
if @@total_retail_price.nil?
@@total_retail_price = self.vehicles.sum(:retail_price)
@@total_retail_price = 0 unless @@total_retail_price
end
@@total_retail_price
end
If I didn''t have to worry about the seller having 0 vehicles I could
just use:
def total_retail_price
@@total_retail_price ||= self.vehicles.sum(:retail_price)
end
Does anyone have any suggestions on how I can fix improve on this?
Thanks,
GiantCranes
-ps, i mistakenly posted this in the ruby forum earlier (sorry)
--
Posted via http://www.ruby-forum.com/.
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---
Giant Cranes wrote:> Hi, > > I need a little help cleaning up the method below. In my .NET days I > would have let this pass, but I am trying to be a good ruby citizen by > keeping things simple. > > This is from a seller model which has many vehicles: > > def total_retail_price > if @@total_retail_price.nil? > @@total_retail_price = self.vehicles.sum(:retail_price) > @@total_retail_price = 0 unless @@total_retail_price > end > @@total_retail_price > end > > If I didn''t have to worry about the seller having 0 vehicles I could > just use: > > def total_retail_price > @@total_retail_price ||= self.vehicles.sum(:retail_price) > end > > Does anyone have any suggestions on how I can fix improve on this?First up, why are you using class variables (the @@ is a class variable)? I''m willing to be you want instance variables (single @). Second up, I don''t think the way you''re summing right now actually works. What you''re passing is the identity, what you want to pass is a block like: self.vehicles.sum { |vehicle| vehicle.retail_price }. Third up, I believe this will actually work in itself without doing anything else. So you should be able to simply do: def total_retail_price @total_retail_price ||= self.vehicles.sum {|vehicle| vehicle.retail_price } end If there are no vehicles this should set total to 0. -- Cheers, - Jacob Atzen --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---
Thanks for your help Jacob, you are correct on each point. -- Posted via http://www.ruby-forum.com/. --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en -~----------~----~----~----~------~----~------~--~---