Greed Dice Game solution

Greed_game_and_score_method.rb

# Greed is a dice game where you roll up to five dice to accumulate
# points.  The following "score" function will be used to calculate the
# score of a single roll of the dice.
#
# A greed roll is scored as follows:
#
# * A set of three ones is 1000 points
#
# * A set of three numbers (other than ones) is worth 100 times the
#   number. (e.g. three fives is 500 points).
#
# * A one (that is not part of a set of three) is worth 100 points.
#
# * A five (that is not part of a set of three) is worth 50 points.
#
# * Everything else is worth 0 points.
#
#
# Examples:
#
# score([1,1,1,5,1]) => 1150 points
# score([2,3,4,6,2]) => 0 points
# score([3,4,5,3,3]) => 350 points
# score([1,5,1,2,4]) => 250 points
#
# More scoring examples are given in the tests below:
#
# Your goal is to write the score method.

def score(dice)
  score = 0
  results = Hash.new

  for x in 1..6 do 
    results[x] = []
  end

  for d in dice do
    results[d].push(d)
  end 

  for key in results.keys do
    if results[key].length >= 3
      if key == 1
        score = 1000
      else
        score = 100 * key
      end
    end
  end

  score += (results[1].length % 3) * 100
  score += (results[5].length % 3) * 50 

  return score 
end

Instantiate hashes with {}

Instead of

  results = Hash.new

do

  results = {}

Use enumerable methods instead of loops

Instead of

  for x in 1..6 do 
    results[x] = []
  end

do

  (1..6).each do |n|
    results[n] = []
  end

Instead of

  for d in dice do
    results[d].push(d)
  end

do

  dice.each do |d|
    results[d].push(d)
  end

etc.

Omit return except to short circuit a procedure

Ruby implicitly returns the value of the last expression in the method.

Instead of

return score

do

score

In your particular example, ending the method with the following will suffice:

def score(dice)
  # ...
  # ...
  score += (results[1].length % 3) * 100
  score + (results[5].length % 3) * 50
end

I might have done something like this:

def score(dice)
  (1..6).inject(0) do |score, d|
    multiplier = d == 1 ? 10 : 1
    count = dice.select { |die| die == d }.count

    if count >= 3
      score += (d * multiplier * 100)
      count -= 3
    end

    if d == 1 || d == 5
      score += (multiplier * count * d * 10)
    end

    score
  end
end

Ah - I hadn't thought about abstracting the multiplier.

Also, I was unaware of passing multiple variables into the do sandwich. I would have a hard time remembering whether d was assigned to the (1..6) or score.

May I also mention that mutating score on each iteration seems like a Haskel thang.