def should_be_upgraded_to_apprentice?
rank = Rank.find_by_id(1)
if self.cbc >= rank.cbc && self.dbc >= rank.dbc &&
self.cbp >= rank.cbp && self.dbp >= rank.dbp &&
self.cbs >= rank.cbs && self.dbs >= rank.dbs &&
self.cpd >= rank.cpd && self.dpd >= rank.dpd &&
self.cpe >= rank.cpe && self.dpe >= rank.dpe &&
self.cpa >= rank.cpa && self.dpa >= rank.dpa &&
self.cnb >= rank.cnb && self.dnb >= rank.dnb &&
self.cad >= rank.cad && self.dad >= rank.dad then true
else
false
end
end
def should_be_upgraded_to_practitioner?
rank = Rank.find_by_id(2)
if self.cbc >= rank.cbc && self.dbc >= rank.dbc &&
self.cbp >= rank.cbp && self.dbp >= rank.dbp &&
self.cbs >= rank.cbs && self.dbs >= rank.dbs &&
self.cpd >= rank.cpd && self.dpd >= rank.dpd &&
self.cpe >= rank.cpe && self.dpe >= rank.dpe &&
self.cpa >= rank.cpa && self.dpa >= rank.dpa &&
self.cnb >= rank.cnb && self.dnb >= rank.dnb &&
self.cad >= rank.cad && self.dad >= rank.dad then true
else
false
end
end
def should_be_upgraded_to_master?
rank = Rank.find_by_id(3)
if self.cbc >= rank.cbc && self.dbc >= rank.dbc &&
self.cbp >= rank.cbp && self.dbp >= rank.dbp &&
self.cbs >= rank.cbs && self.dbs >= rank.dbs &&
self.cpd >= rank.cpd && self.dpd >= rank.dpd &&
self.cpe >= rank.cpe && self.dpe >= rank.dpe &&
self.cpa >= rank.cpa && self.dpa >= rank.dpa &&
self.cnb >= rank.cnb && self.dnb >= rank.dnb &&
self.cad >= rank.cad && self.dad >= rank.dad then true
else
false
end
end
def should_be_upgraded_to_designer?
rank = Rank.find_by_id(4)
if self.cbc >= rank.cbc && self.dbc >= rank.dbc &&
self.cbp >= rank.cbp && self.dbp >= rank.dbp &&
self.cbs >= rank.cbs && self.dbs >= rank.dbs &&
self.cpd >= rank.cpd && self.dpd >= rank.dpd &&
self.cpe >= rank.cpe && self.dpe >= rank.dpe &&
self.cpa >= rank.cpa && self.dpa >= rank.dpa &&
self.cnb >= rank.cnb && self.dnb >= rank.dnb &&
self.cad >= rank.cad && self.dad >= rank.dad then true
else
false
end
end
def should_be_upgraded_to_engineer?
rank = Rank.find_by_id(5)
if self.cbc >= rank.cbc && self.dbc >= rank.dbc &&
self.cbp >= rank.cbp && self.dbp >= rank.dbp &&
self.cbs >= rank.cbs && self.dbs >= rank.dbs &&
self.cpd >= rank.cpd && self.dpd >= rank.dpd &&
self.cpe >= rank.cpe && self.dpe >= rank.dpe &&
self.cpa >= rank.cpa && self.dpa >= rank.dpa &&
self.cnb >= rank.cnb && self.dnb >= rank.dnb &&
self.cad >= rank.cad && self.dad >= rank.dad then true
else
false
end
end
def should_be_upgraded_to_architect?
rank = Rank.find_by_id(6)
if self.cbc >= rank.cbc && self.dbc >= rank.dbc &&
self.cbp >= rank.cbp && self.dbp >= rank.dbp &&
self.cbs >= rank.cbs && self.dbs >= rank.dbs &&
self.cpd >= rank.cpd && self.dpd >= rank.dpd &&
self.cpe >= rank.cpe && self.dpe >= rank.dpe &&
self.cpa >= rank.cpa && self.dpa >= rank.dpa &&
self.cnb >= rank.cnb && self.dnb >= rank.dnb &&
self.cad >= rank.cad && self.dad >= rank.dad then true
else
false
end
end
Refactorings
No refactoring yet !
bakkdoor
August 13, 2008, August 13, 2008 02:15, permalink
1 rating.
Login to rate!
is it actually just the name of the method and the id? it looks like it...
this could help:
# creates those methods for you
def should_be_upgraded_to(title, id)
define_method "should_be_upgraded_to_#{title}?" do
rank = Rank.find_by_id(id)
if self.cbc >= rank.cbc && self.dbc >= rank.dbc &&
self.cbp >= rank.cbp && self.dbp >= rank.dbp &&
self.cbs >= rank.cbs && self.dbs >= rank.dbs &&
self.cpd >= rank.cpd && self.dpd >= rank.dpd &&
self.cpe >= rank.cpe && self.dpe >= rank.dpe &&
self.cpa >= rank.cpa && self.dpa >= rank.dpa &&
self.cnb >= rank.cnb && self.dnb >= rank.dnb &&
self.cad >= rank.cad && self.dad >= rank.dad then true
else
false
end
end
end
# create them like this in your class:
should_be_upgraded_to :apprentice, 1
should_be_upgraded_to :practitioner, 2
should_be_upgraded_to :master, 3
should_be_upgraded_to :designer, 4
should_be_upgraded_to :engineer, 5
should_be_upgraded_to :architect, 6
# i hope it works, haven't actually tried it out...
bakkdoor
August 13, 2008, August 13, 2008 02:20, permalink
1 rating.
Login to rate!
make that a class method, the way it is written right now..
like so:
# creates those methods for you
def self.should_be_upgraded_to(title, id)
define_method "should_be_upgraded_to_#{title}?" do
rank = Rank.find_by_id(id)
if self.cbc >= rank.cbc && self.dbc >= rank.dbc &&
self.cbp >= rank.cbp && self.dbp >= rank.dbp &&
self.cbs >= rank.cbs && self.dbs >= rank.dbs &&
self.cpd >= rank.cpd && self.dpd >= rank.dpd &&
self.cpe >= rank.cpe && self.dpe >= rank.dpe &&
self.cpa >= rank.cpa && self.dpa >= rank.dpa &&
self.cnb >= rank.cnb && self.dnb >= rank.dnb &&
self.cad >= rank.cad && self.dad >= rank.dad then true
else
false
end
end
end
# create them like this in your class:
should_be_upgraded_to :apprentice, 1
should_be_upgraded_to :practitioner, 2
should_be_upgraded_to :master, 3
should_be_upgraded_to :designer, 4
should_be_upgraded_to :engineer, 5
should_be_upgraded_to :architect, 6
# i hope it works, haven't actually tried it out...
leemic
August 13, 2008, August 13, 2008 03:02, permalink
2 ratings.
Login to rate!
You can also simplify if statement
def should_be_upgraded_to(title, id)
define_method "should_be_upgraded_to_#{title}?" do
rank = Rank.find_by_id(id)
[ :cbc , :dbc , :cbp , :dbp , :cbs , :dbs , :cpd , :dpd ,
:cpe , :dpe , :cpa , :dpa , :cnb , :dnb , :cad , :dad
].each do | sym |
return false unless ( self.send( sym ) >= rank.send( sym ) )
end
return true
end
end
Ryan Briones
August 13, 2008, August 13, 2008 05:15, permalink
2 ratings.
Login to rate!
def should_be_upgraded_to_apprentice?
rank = Rank.find_by_id(1)
should_be_upgraded?(rank)
end
def should_be_upgraded_to_practitioner?
rank = Rank.find_by_id(2)
should_be_upgraded?(rank)
end
def should_be_upgraded_to_master?
rank = Rank.find_by_id(3)
should_be_upgraded?(rank)
end
def should_be_upgraded_to_designer?
rank = Rank.find_by_id(4)
should_be_upgraded?(rank)
end
def should_be_upgraded_to_engineer?
rank = Rank.find_by_id(5)
should_be_upgraded?(rank)
end
def should_be_upgraded_to_architect?
rank = Rank.find_by_id(6)
should_be_upgraded?(rank)
end
private
def should_be_upgraded?(rank)
return self.cbc >= rank.cbc && self.dbc >= rank.dbc &&
self.cbp >= rank.cbp && self.dbp >= rank.dbp &&
self.cbs >= rank.cbs && self.dbs >= rank.dbs &&
self.cpd >= rank.cpd && self.dpd >= rank.dpd &&
self.cpe >= rank.cpe && self.dpe >= rank.dpe &&
self.cpa >= rank.cpa && self.dpa >= rank.dpa &&
self.cnb >= rank.cnb && self.dnb >= rank.dnb &&
self.cad >= rank.cad && self.dad >= rank.dad && true
end
George
August 13, 2008, August 13, 2008 11:41, permalink
1 rating.
Login to rate!
I might be missing the point here, but can't you just do the one method and pass in the rank to evaluate? I've also "borrowed" the refactored if statement from leemic if that's okay!
def should_be_upgraded?(rank)
[ :cbc , :dbc , :cbp , :dbp , :cbs , :dbs , :cpd , :dpd ,
:cpe , :dpe , :cpa , :dpa , :cnb , :dnb , :cad , :dad
].each do | sym |
return false unless ( self.send( sym ) >= rank.send( sym ) )
end
return true
end
# then object can be evaluated for upgrade by (if you can find the rank by name, otherwise find it by id)
obj.should_be_upgraded?(Rank.find_by_name("architect"))
George
August 13, 2008, August 13, 2008 11:41, permalink
No rating.
Login to rate!
I might be missing the point here, but can't you just do the one method and pass in the rank to evaluate? I've also "borrowed" the refactored if statement from leemic if that's okay!
def should_be_upgraded?(rank)
[ :cbc , :dbc , :cbp , :dbp , :cbs , :dbs , :cpd , :dpd ,
:cpe , :dpe , :cpa , :dpa , :cnb , :dnb , :cad , :dad
].each do | sym |
return false unless ( self.send( sym ) >= rank.send( sym ) )
end
return true
end
# then object can be evaluated for upgrade by (if you can find the rank by name, otherwise find it by id)
obj.should_be_upgraded?(Rank.find_by_name("architect"))
kmcd
August 13, 2008, August 13, 2008 12:46, permalink
1 rating.
Login to rate!
Most comments are in the code. Obviously I couldn't run this.
I'm not sure if the job levels are stored as 'name'. If they are, you could eliminate the JOB_LEVELS hash. Less code === better code.
Further refactoring of the extended boolean expression significantly reduces the readability IMHO.
JOB_LEVELS = {
:apprentice => 1,
:practicioner => 2,
:master => 3,
:designer => 4,
:engineer => 5,
:architect => 6
}
# Usage: upgrade_to?(:apprentice)
# => true
def upgrade_to?(level)
rank = Rank.find_by_id JOB_LEVELS[level]
# If/else not needed as this expression will evaluate to true/false
self.cbc >= rank.cbc && self.dbc >= rank.dbc &&
self.cbp >= rank.cbp && self.dbp >= rank.dbp &&
self.cbs >= rank.cbs && self.dbs >= rank.dbs &&
self.cpd >= rank.cpd && self.dpd >= rank.dpd &&
self.cpe >= rank.cpe && self.dpe >= rank.dpe &&
self.cpa >= rank.cpa && self.dpa >= rank.dpa &&
self.cnb >= rank.cnb && self.dnb >= rank.dnb &&
self.cad >= rank.cad && self.dad >= rank.dad
end
# Further refactoring
# rank = Rank.find_by_id JOB_LEVELS[level]
# Could also be written as: (eliminating the need for JOB_LEVELS)
# rank = Rank.find_by_name level.to_s
# Assuming that the job level is stored in the 'name' column
# self.cbc >= rank.cbc && self.dbc >= rank.dbc && ...
# The extended boolean expression could be refactored to the following:
# (There is a reduction in readability here however.)
# vars = %w[ bc bp bs pd pe pa nb ad ].map { |var| [ "c" + var, "d" + var ]}.flatten
#
# expr = vars.inject("") do |accum,var|
# accum = "self.#{var} >= rank.#{var} &&"
# end.chomp "&&"
#
# eval(expr)
Adam
August 13, 2008, August 13, 2008 14:44, permalink
2 ratings.
Login to rate!
class Rank < ActiveRecord::Base
COMPARATORS = %w[cbc dbc cbp dbp cbs dbs cpd dpd
cpe dpe cpa dpa cnb dnb cad dad]
def >=(rank)
COMPARATORS.all? do |comparitor|
send(comparitor) >= rank.send(comparitor)
end
end
def method_missing(method, *args, &block)
if method.to_s =~ /^should_be_upgraded_to_(.*)\?/
self >= self.class.find_by_name($1)
else
super
end
end
end
I have 6 methods that are almost identical accept for the call of Rank,find_by_id. I need to check data in the ranks table relative to what the upgrade should be. Any help refactoring these methods would be great. Thanks in advance!