# Need help cleaning up a model method

**URL:** <https://www.ruby-forum.com/t/need-help-cleaning-up-a-model-method/112025>\
**Category:** Rails\
**Created:** [August 14, 2007, 12:40pm UTC](https://www.ruby-forum.com/t/need-help-cleaning-up-a-model-method/112025 "2007-08-14T12:40:35Z")\
**Posts on this page:** 3\
**Page:** 1

<div class="post-metadata">

**Author:** ![giantcranes](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/g/ecd19e/32.png) [@giantcranes](https://www.ruby-forum.com/u/giantcranes)\
**Post date:** [August 14, 2007, 12:40pm UTC](https://www.ruby-forum.com/t/need-help-cleaning-up-a-model-method/112025/1 "2007-08-14T12:40:35Z")

</div>

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)

---

<div class="post-metadata">

**Author:** ![giantcranes](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/g/ecd19e/32.png) [@giantcranes](https://www.ruby-forum.com/u/giantcranes)\
**Post date:** [August 14, 2007, 1:41pm UTC](https://www.ruby-forum.com/t/need-help-cleaning-up-a-model-method/112025/2 "2007-08-14T13:41:48Z")

</div>

Giant C. wrote:

> ```
> @@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 A.

---

<div class="post-metadata">

**Author:** ![giantcranes](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/g/ecd19e/32.png) [@giantcranes](https://www.ruby-forum.com/u/giantcranes)\
**Post date:** [August 14, 2007, 7:43pm UTC](https://www.ruby-forum.com/t/need-help-cleaning-up-a-model-method/112025/3 "2007-08-14T19:43:21Z")

</div>

Thanks for your help Jacob, you are correct on each point.