[Ruby] On Similar Methods Refactoring

2008-08-13T12:46:14-07:00

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)

kmcd

[Ruby] On Similar Methods Refactoring