def planned_percent_complete
if self.planned_complete_in_dollars != nil
(( self.planned_complete_in_dollars.to_f / self.task.budget.to_f ) * 100).round(2)
else
0
end
end
def planned_percent_complete=(ppc)
self.planned_complete_in_dollars = (ppc.to_i / 100.0) * self.task.budget
end
def actual_percent_complete
if self.actual_completed_in_dollars != nil
(( self.actual_completed_in_dollars.to_f / self.task.budget.to_f ) * 100).round(2)
else
0
end
end
def actual_percent_complete=(ppc)
self.actual_complete_in_dollars = (ppc.to_i / 100.0) * self.task.budget
end
Refactorings
No refactoring yet !
Derek Perrault
January 8, 2008, January 08, 2008 19:35, permalink
My method would require you to change #actual_completed_in_dollars to #actual_complete_in_dollars so it mirrors the #planned_complete_in_dollars name.
Also, I didn't know what to call the method I extracted in the "setters". Since you know the domain, you can give it a good name.
def planned_percent_complete
percent_complete :planned
end
def actual_percent_complete
percent_complete :actual
end
def planned_percent_complete=(ppc)
self.planned_complete_in_dollars = good_name_for_this(ppc)
end
def actual_percent_complete=(ppc)
self.actual_complete_in_dollars = good_name_for_this(ppc)
end
private
def percent_complete(which)
self.send("#{which}_complete_in_dollars").nil? ? ((self.send("#{which}_complete_in_dollars).to_f / self.task.budget.to_f) * 100).round(2) : 0
end
def good_name_for_this(ppc)
(ppc.to_i / 100.0) * self.task.budget
end
Derek Perrault
January 8, 2008, January 08, 2008 19:39, permalink
*sigh*
I forgot to close a string. I noticed when it hit the syntax highlighter.
Here it is all fixed up.
def planned_percent_complete
percent_complete :planned
end
def actual_percent_complete
percent_complete :actual
end
def planned_percent_complete=(ppc)
self.planned_complete_in_dollars = good_name_for_this(ppc)
end
def actual_percent_complete=(ppc)
self.actual_complete_in_dollars = good_name_for_this(ppc)
end
private
def percent_complete(which)
self.send("#{which}_complete_in_dollars").nil? ? ((self.send("#{which}_complete_in_dollars").to_f / self.task.budget.to_f) * 100).round(2) : 0
end
def good_name_for_this(ppc)
(ppc.to_i / 100.0) * self.task.budget
end
Dan Kubb
January 8, 2008, January 08, 2008 20:06, permalink
Here's an alternate approach that dynamically defines the methods.
[ :planned, :actual ].each do |type|
define_method("#{type}_percent_complete") do
return 0 if send("#{type}_complete_in_dollars").nil?
((send("#{type}_complete_in_dollars").to_f / task.budget.to_f) * 100).round(2)
end
define_method("#{type}_percent_complete=") do |ppc|
send "#{type}_complete_in_dollars=", (ppc.to_i / 100.0) * task.budget
end
end
derekperrault.wordpress.com
January 8, 2008, January 08, 2008 20:23, permalink
My refactoring above had a bug. The ?: return expressions were out of order. Finally, I extracted the dynamic #send so as to be more dry.
I'm done, really.
(and I couldn't edit until I hooked up my actual login... sorry bout the multiple posts)
def planned_percent_complete
percent_complete :planned
end
def actual_percent_complete
percent_complete :actual
end
def planned_percent_complete=(ppc)
self.planned_complete_in_dollars = good_name_for_this(ppc)
end
def actual_percent_complete=(ppc)
self.actual_complete_in_dollars = good_name_for_this(ppc)
end
private
def percent_complete(which)
complete_in_dollars(which).nil? ? 0 : (complete_in_dollars(which).to_f / self.task.budget.to_f) * 100).round(2)
end
def good_name_for_this(ppc)
(ppc.to_i / 100.0) * self.task.budget
end
def complete_in_dollars(which)
self.send("#{which}_complete_in_dollars")
end
Chris W.
January 9, 2008, January 09, 2008 01:39, permalink
Here is the final version I ended up with. It was a minor tweak on the dynamic define. I had to create the hash since my method names had some verb tense changes.
# dynamically define the methods to get/set based on percent complete for both the
# actual and planned values
methods = { :planned => 'complete', :actual => 'completed' }
methods.each do |type, verb|
define_method("#{type}_percent_complete") do
return 0 if send("#{type}_#{verb}_in_dollars").nil?
((send("#{type}_#{verb}_in_dollars").to_f / task.budget.to_f) * 100).round(2)
end
define_method("#{type}_percent_complete=") do |pc|
send "#{type}_#{verb}_in_dollars=", (pc.to_i / 100.0) * task.budget
end
end
Charles
January 9, 2008, January 09, 2008 06:17, permalink
Note that nil.to_f is 0.0, so you can probably remove the special case too
I know there has to be a "ruby" way to remove the duplicate methods. The logic is the same, just the ActiveRecord attribute changes. I tried a few things but my unit tests failed.
Thanks