12f51e0e2ea40d6fb0df1c64324962a0

I'm having fun going through the Ruby Koans ( http://github.com/edgecase/ruby_koans ), a series of test cases to be solved. It includes the exercise to write a method "score" for the dice game below. My code works, but doesn't feel "efficient"/idiomatic.

The question is: what are the best practises for a method with all kind of checks? How can I avoid countless if- or case-statements?

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
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
require 'edgecase'

# 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
#
# More scoing examples are given in the tests below:
#
# Your goal is to write the score method.

def score(dice)
  points = 0
  dice_freq = [0, 0, 0, 0, 0, 0, 0]

  # the dice_freq array collects the frequency of each number;
  # the number is the array index, the value is the frequency
  # of the number
  dice.each {|d| dice_freq[d] += 1 }

  # 3 times 1 scores 1000
  if dice_freq[1] >= 3
      points += 1000
      dice_freq[1] -= 3
    end

  # 3 times another value scores 100
  dice_freq.each_with_index do |freq, index|
    if dice_freq[index] >= 3
      points += index * 100
      dice_freq[index] -= 3
    end
  end

  # count remaining 1s and 5s
  points += dice_freq[1] * 100 if dice_freq[1] > 0
  points += dice_freq[5] * 50  if dice_freq[5] > 0

  points
end

class AboutScoringAssignment < EdgeCase::Koan
  def test_score_of_an_empty_list_is_zero
    assert_equal 0, score([])
  end

  def test_score_of_a_single_roll_of_5_is_50
    assert_equal 50, score([5])
  end

  def test_score_of_a_single_roll_of_1_is_100
    assert_equal 100, score([1])
  end

  def test_score_of_mulitple_1s_and_5s_is_the_sum
    assert_equal 300, score([1,5,5,1])
  end

  def test_score_of_single_2s_3s_4s_and_6s_are_zero
    assert_equal 0, score([2,3,4,6])
  end

  def test_score_of_a_triple_1_is_1000
    assert_equal 1000, score([1,1,1])
  end

  def test_score_of_other_triples_is_100x
    assert_equal 200, score([2,2,2])
    assert_equal 300, score([3,3,3])
    assert_equal 400, score([4,4,4])
    assert_equal 500, score([5,5,5])
    assert_equal 600, score([6,6,6])
  end

  def test_score_of_mixed_is_sum
    assert_equal 250, score([2,5,2,2,3])
    assert_equal 550, score([5,5,5,5])
  end

end

Refactorings

No refactoring yet !

8f6f95c4bd64d5f10dfddfdcd03c19d6

Rick DeNatale

September 23, 2009, September 23, 2009 17:28, permalink

No rating. Login to rate!
12f51e0e2ea40d6fb0df1c64324962a0

sohooo

September 24, 2009, September 24, 2009 08:23, permalink

No rating. Login to rate!

@Rick DeNatale

The rule at line 25 indicates that three equal numbers doesn't have to appear in consecutive order to score 3x <number>.
That of course means that the assertion at line 96 (in my code sample; line 88 in yours) had to be edited. Your usage of "each_cons" would be wrong in that case, if we go by the rules. (However, if we only look at the assertions, it would be ok :)

Bcd38c655b6d2870194b0ef41575c75b

alech

September 24, 2009, September 24, 2009 19:34, permalink

No rating. Login to rate!

sohooo, thanks a lot for pointing me in the direction of the Ruby Koans, I am new to Ruby and am enjoying them as well now. Below is my solution. And yes, I believe the assertion does contradict the rules and examples, too ...

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
def score(dice)
    score = 0
    (1..6).each do |n|
        score += triple_score n if triple_present? dice, n
    end
    score += rest_occurences(dice, 1) * 100
    score += rest_occurences(dice, 5) * 50
    score
end

def occurences(dice, number)
    dice.select { |d| d == number }.size
end

def triple_present?(dice, number)
    occurences(dice, number) >= 3
end

def triple_score(number)
    case number
    when 1
        1000
    else
        100 * number
    end
end

def rest_occurences(dice, number)
    o = occurences(dice, number)
    if o >= 3 then
        o - 3 # 3 of them have been accounted for as a set
    else
        o
    end
end
220a1ce7a99b513ece2aca0f6d4688c7

zetafish

September 24, 2009, September 24, 2009 23:10, permalink

No rating. Login to rate!

Hey sohoo, nice tutorial problems on ruby there. Here a regex solution.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
S = [    [/111/, 1000],
         [/222/,  200],
         [/333/,  300],
         [/444/,  400],
         [/555/,  500],
         [/666/,  600],
         [/1/,    100],
         [/5/,     50]
    ]

def re_score(s)
  p, n = S.find{ |x| s =~ x.first }
  p.nil? ? 0 : n + re_score($` + $')
end

def score(dice)
  re_score dice.sort.collect{|n|n.to_s}.reduce(:+)
end
C1f7bc8064161e7408ef62d97bb636ac

Mortice

September 25, 2009, September 25, 2009 13:49, permalink

No rating. Login to rate!

My best attempt, not sure if it can be done in one block...

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
def score(dice)
  total = 0
  dice.sort!
  dice.each_with_index do |die, index|
    if die == dice[index + 1] and die == dice[index + 2] then
      total += if die == 1 then 1000 else die * 100 end
      3.times { dice.shift }
    end
  end
  dice.inject(total) do |sum, die|
    sum += case die
      when 5 then 50
      when 1 then 100
      else 0
    end
  end  
end
5fa7995da0a33e2c6286f6fe734f39a5

Alex O

October 17, 2009, October 17, 2009 10:03, permalink

No rating. Login to rate!

Here is mine, which is basically a highly condensed version of yours. It follows the specification's logic pretty much exactly, as yours does.

1
2
3
4
5
6
7
def score(dice)
  count = Array.new(6) {|n| dice.select{|i| i-1 == n}.length}
  score = count[0] / 3 * 1000
  (1..5).each {|i| score += count[i] / 3 * 100 * (i+1) }
  score += count[0] % 3 * 100
  score += count[4] % 3 * 50
end
Avatar

Desty Nova

October 31, 2009, October 31, 2009 01:55, permalink

No rating. Login to rate!
1
2
3
4
5
6
7
8
9
10
def triple(die); die == 1 ? 1000 : die * 100; end
def single(die); die == 1 ? 100 : die == 5 ? 50 : 0; end
def score(array)
  array.sort!
  score= 0
  until array.empty?
    score+= array[0] == array[2] ? triple(array.slice!(0..2).first) : single(array.slice!(0))
  end
  score
end

Your refactoring





Format Copy from initial code

or Cancel