def sync_scores
home_score = Score.find_or_initialize_by_team_id_and_game_id(home_team.id, id)
visiting_score = Score.find_or_initialize_by_team_id_and_game_id(visiting_team.id, id)
if home_team_score.nil? || visiting_team_score.nil?
return
end
if home_team_score > visiting_team_score
home_score.score = Score::WIN
visiting_score.score = Score::LOSS
elsif visiting_team_score > home_team_score
home_score.score = Score::LOSS
visiting_score.score = Score::WIN
elsif visiting_team_score == home_team_score
home_score.score = Score::TIE
visiting_score.score = Score::TIE
end
home_score.save!
visiting_score.save!
end
Refactorings
No refactoring yet !
EchoLima
June 26, 2011, June 26, 2011 16:10, permalink
No rating.
Login to rate!
You're comparing the same two values and assigning values to the same two attributes. At best you can abstract that comparison into the three comparison operators you're using, and the outcomes associated with them. Iterate through that set until we reach the comparison operator that returns true when used on the two values, then apply the associated outcome to the two attributes. (Are home_team_score and home_team.score meant to be different, and what does either represent?)
If this code is in the Score class itself then you don't need to quantify the class name on your constants.
{:> => [WIN, LOSS], :< => [LOSS, WIN], :== => [TIE, TIE]}.each do |operator, result|
if home_team_score.send(operator, visiting_team_score)
home_team.score, visiting_team.score = result
break
end
end
steved
June 26, 2011, June 26, 2011 16:32, permalink
No rating.
Login to rate!
class Game
def sync_cores
return if home_team_score.nil? || visiting_team_score.nil?
Score.find_or_create_score(id, home_team.id, home_team_score, visiting_team_score)
Score.find_or_create_score(id, visiting_team.id, visiting_team_score, home_team_score)
end
end
class Score
SCORE_HASH = { -1 => LOSS, 0 => TIE, 1 => WIN } # <=> returns -1, 0, or 1
class << self
def find_or_create_score(game_id, team_id, team_score, other_team_score)
score = find_or_initialize_by_game_id_and_team_id(game_id, team_id)
score.score = SCORE_HASH[team_score <=> other_team_score]
score.save!
end
end
end
This method records an update to a game and syncs the scores on the two teams involved. I'm looking for a way to cleanup that ugly if/elsif block as well as a more elegant way of tallying scores for the teams. Score::WIN, etc are constants on the Score class