D7b8e44d9c430f24a9426725d4c33480

# Greed is a dice game where you roll up to five dice to accumulate
# points. The following "score" function will be used 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

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
def score(dice)
  
  count = [0,0,0,0,0,0]
  
  dice.each { |i|
    count[0] += 1 if i == 1
    count[1] += 1 if i == 2
    count[2] += 1 if i == 3
    count[3] += 1 if i == 4
    count[4] += 1 if i == 5
    count[5] += 1 if i == 6
  }

  sum = 0

  unless dice  == []

    if count[0] < 3
      sum += count[0] * 100
    elsif count[0] == 3
      sum += 1000
    elsif count[0] > 3
      sum  = 1000 + (count[0] -3) * 100
    end

    (1..5).each { |i|
     if count[i] == 3
      sum +=  (i+1) * 100
     end
    }

    if count[4] < 3
      sum += count[4] * 50
    elsif count[4] > 3
      sum  = 500 + (count[4] -3) * 50
    end
    
  end

  return sum

end

Refactorings

No refactoring yet !

Avatar

Amit

February 3, 2010, February 03, 2010 14:40, permalink

No rating. Login to rate! 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
def score(dice)
  
  count = [0,0,0,0,0,0]
  
  dice.each { |i|
    (1..6).each do |j|
      count[j-1] += 1 if i == j
    end
  }

  sum = 0

  unless dice  == []

    if count[0] < 3
      sum += count[0] * 100
    elsif count[0] == 3
      sum += 1000
    elsif count[0] > 3
      sum  = 1000 + (count[0] -3) * 100
    end

    (1..5).each do |i|
     if count[i] == 3
      sum +=  (i+1) * 100
     end
    end

    if count[4] < 3
      sum += count[4] * 50
    elsif count[4] > 3
      sum  = 500 + (count[4] -3) * 50
    end
    
  end

  return sum

end
Avatar

OP

February 3, 2010, February 03, 2010 19:05, permalink

No rating. Login to rate!

Even shorter...

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
def score(dice)
  sum = 0
  count = [0,0,0,0,0,0]

  dice.each { |i| count[i-1] += 1 }

  # Three ones is 1000 points
  sum += 1000 and count[0] -= 3 if count[0] >= 3

  # 100 times the number for three equal numbers
  count.each_with_index { |n, i| sum += n * 100 and count[i] -= 3 if n == 3 }

  # 100 points per leftover one
  sum += count[0] * 100
  
  # 50 points per leftover five
  sum += count[4] * 50
end
F9a9ba6663645458aa8630157ed5e71e

Ants

February 3, 2010, February 03, 2010 21:21, permalink

No rating. Login to rate!

Either there is something missing in the rules/spec, or there is a bug in the original implementation.

The rules say "A set of three numbers (other than ones) is worth 100 times the number."
and this has been coded by:
Websymphony: line 27-29
Amit: line 24-26
OP: line 11

If the input dice is [2,2,2,2,4] it looks like the score should be 3 * 2 * 100 = 600 because there are 3 2's that can build up the set, and the extra 2 and 4 can be discarded. All the implementations above look like they will score 0 because count[1] == 4, while the check is only for count[1] == 3.

Am I misreading the rules, or is there a bug in the implementation? I'm not sure because the rules specified in http://en.wikipedia.org/wiki/Greed_(dice_game) are different.

Avatar

Amit

February 4, 2010, February 04, 2010 16:52, permalink

No rating. Login to rate!

Yes, there was a bug in the original implementation. Missed out those checks.
But for the input [2,2,2,2,4] output would be 2*100 = 200 not 3*2*100 = 600.
The rule says "A set of three numbers (other than ones) is worth 100 times the number."
It is slightly modified dice game, not same as given in http://en.wikipedia.org/wiki/Greed_(dice_game).

OP's Implementation fails for many cases For eg. [5,1,4,4,4] gives 450 where as it should have been 4*100 + 1*100 + 50 = 550.
[5,6,6,6,4] gives 350 where as output should have been 50 + 6*100 = 650

Here is my fixed code.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
def score(dice)

count = [0,0,0,0,0,0]
dice.each { |i| count[i-1] += 1 }
puts count
  sum = 0
unless dice  == []
  if count[0] < 3
    sum = sum + count[0] * 100
  elsif count[0] == 3
    sum = sum + 1000    
  elsif count[0] > 3
    sum  = 1000 + (count[0] -3) * 100
  end

  (1..5).each { |i|
   if count[i] == 3
    sum = sum +  (i+1) * 100
   end
  }
  
 if count[1] > 3 
   sum = sum + 200
 end
 if count[2] > 3 
   sum = sum + 300
 end
 if count[3] > 3 
   sum = sum + 400
 end
 if count[5] > 3 
   sum = sum + 600
 end
  

  if count[4] < 3
    sum = sum + count[4] * 50
  elsif count[4] > 3
    sum  = 500 + (count[4] -3) * 50
  end
end

return sum

end
Avatar

WebSymphony

February 4, 2010, February 04, 2010 17:29, permalink

No rating. Login to rate!

Even Shorter!

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
def score(dice)
  sum = 0
  count = [0,0,0,0,0,0]

  dice.each { |i| count[i-1] += 1 }
    
  unless dice  == []

    if count[0] < 3
      sum += count[0] * 100
    elsif count[0] == 3
      sum += 1000
    elsif count[0] > 3
      sum  = 1000 + (count[0] -3) * 100
    end

    (1..5).each { |i|
      if count[i] == 3
        sum +=  (i+1) * 100
      end
    }

    [1,2,3,5].each { |i| sum+= (i+1)*100 if count[i] > 3 }

    if count[4] < 3
      sum += count[4] * 50
    elsif count[4] > 3
      sum  = 500 + (count[4] -3) * 50
    end

  end

  return sum
end
Avatar

OP

February 4, 2010, February 04, 2010 22:24, permalink

1 rating. Login to rate!

According to http://en.wikipedia.org/wiki/Greed_(dice_game) i believe [2,2,2,2,4] should be 2 * 100 = 200 for the first three 2's, then doubled because of the extra 2, making it 400.

Line 13 now says (i+1) * 100 instead of n * 100, fixing the bug pointed out by Amit.
Line 14 would result in the score being calculated according to the above URL. The line is commented out, however, to rather produce 200 for an input of [2,2,2,2,4].

And it's still shorter ;)

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
def score(dice)
  sum = 0
  count = [0,0,0,0,0,0]

  dice.each { |i| count[i-1] += 1 }

  # Three ones is 1000 points
  sum += 1000 and count[0] -= 3 if count[0] >= 3

  # 100 times the number for the first three equal numbers, doubled once per extra
  count.each_with_index do |n, i|
    triple_sum = 0
    triple_sum = (i+1) * 100 and count[i] -= 3 if n >= 3
    # count[i].times { triple_sum *= 2 }
    sum += triple_sum
  end

  # 100 points per leftover one
  sum += count[0] * 100

  # 50 points per leftover five
  sum += count[4] * 50
end
F9a9ba6663645458aa8630157ed5e71e

Ants

February 4, 2010, February 04, 2010 23:08, permalink

No rating. Login to rate!

Nice! Good job catching my misinterpretation/miscalculation of the "set of 3" rule. I'm glad that you understood what I was trying to point out.

All the refactorings afterwards to take that into account look great!

96bfee99a277135b44af65de46150f3f

Alex Baranosky

February 8, 2010, February 08, 2010 06:57, permalink

No rating. Login to rate!

Here's my second iteration of refactoring :) 

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
class GreedGame
  SET_OF_THREE_POINT_MULTIPLIER = 100
  POINTS_FOR_ONE = 100
  POINTS_FOR_FIVE = 50
  THREE_ONES_BONUS_POINTS = 900

  def score_the_dice(dice = [])
    raise Exception('can only enter 5 dice at most') if dice.length > 5
    return 0 if dice == []

    score = 0
    dice_tally = tally_dice(dice)
    score += score_ones_through_sixes(dice_tally)
    score += score_ones_special_case(dice_tally[1])
    score += score_fives_special_case(dice_tally[5])
    score
  end

  private

  def tally_dice(dice)
    dice_tally = { 1 => 0, 2 => 0, 3 => 0, 4 => 0, 5 => 0, 6 => 0 }
    dice.each_with_object(dice_tally) do |die_value, tally|
      tally[die_value] += 1
    end
  end

  def score_ones_through_sixes(dice_tally)
    (1..6).each { |num| return num * SET_OF_THREE_POINT_MULTIPLIER if dice_tally[num] >= 3 }
    0
  end

  def score_ones_special_case(ones_rolled)
    score = ones_rolled >= 3 ? THREE_ONES_BONUS_POINTS : 0
    score += score_special_case(ones_rolled, POINTS_FOR_ONE)
  end

  def score_fives_special_case(fives_rolled)
    score_special_case(fives_rolled, POINTS_FOR_FIVE)
  end

  def score_special_case(num_rolled, points_for_die)
    if num_rolled < 3
      num_rolled * points_for_die
    elsif num_rolled > 3
      extra_rolls(num_rolled) * points_for_die
    else
      0
    end
  end

  def extra_rolls(times_rolled)
    times_rolled - 3
  end

end
96bfee99a277135b44af65de46150f3f

Alex Baranosky

February 8, 2010, February 08, 2010 07:37, permalink

No rating. Login to rate!

3rd and final refactoring:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
class GreedGame
  THREE_OF_A_KIND_POINT_MULTIPLIER = 100
  POINTS_FOR_ONE = 100
  POINTS_FOR_FIVE = 50
  THREE_ONES_BONUS_POINTS = 900

  def score_the_dice(dice = [])
    raise Exception('can only enter 5 dice at most') if dice.length > 5

    dice_tally = tally_dice(dice)

    @score = 0
    score_three_of_a_kind(dice_tally)
    score_ones_special_case(dice_tally[1])
    score_fives_special_case(dice_tally[5])
    @score
  end

  private

  def tally_dice(dice)
    dice_tally = { 1 => 0, 2 => 0, 3 => 0, 4 => 0, 5 => 0, 6 => 0 }
    dice.each_with_object(dice_tally) do |die_value, tally|
      tally[die_value] += 1
    end
  end

  def score_three_of_a_kind(dice_tally)
    (1..6).each do |die_value|
      @score += die_value * THREE_OF_A_KIND_POINT_MULTIPLIER if dice_tally[die_value] >= 3
    end
  end

  def score_ones_special_case(ones_rolled)
    @score += ones_rolled >= 3 ? THREE_ONES_BONUS_POINTS : 0
    score_special_case(ones_rolled, POINTS_FOR_ONE)
  end

  def score_fives_special_case(fives_rolled)
    score_special_case(fives_rolled, POINTS_FOR_FIVE)
  end

  def score_special_case(num_rolled, points_for_die)
    @score +=
            if num_rolled < 3
              num_rolled * points_for_die
            elsif num_rolled > 3
              extra_rolls(num_rolled) * points_for_die
            else
              0
            end
  end

  def extra_rolls(times_rolled)
    times_rolled - 3
  end

end

Your refactoring





Format Copy from initial code

or Cancel