Skip to main content
edited tags
Link
RubberDuck
  • 30.9k
  • 6
  • 71
  • 174
deleted 648 characters in body
Source Link
Jamal
  • 34.9k
  • 13
  • 133
  • 237

EDIT: The updated method

def score(dice)
  patterns = {[1,1,1]=>1000, [2,2,2]=>200, [3,3,3]=>300, [4,4,4]=>400,
              [5,5,5]=>500,  [6,6,6]=>600, 1=>100, 5=>50}
  sorted = dice.sort

  triple = patterns[sorted[0..2]]
  single = patterns[sorted[0]]
  if triple
    partial_score = triple
    rest = sorted[3..-1]
  elsif single
    partial_score = single
    rest = sorted[1..-1]
  else
    partial_score = 0
    rest = sorted[1..-1]
  end

  if rest
    partial_score + score(rest) 
  else
    partial_score
  end
end

EDIT: The updated method

def score(dice)
  patterns = {[1,1,1]=>1000, [2,2,2]=>200, [3,3,3]=>300, [4,4,4]=>400,
              [5,5,5]=>500,  [6,6,6]=>600, 1=>100, 5=>50}
  sorted = dice.sort

  triple = patterns[sorted[0..2]]
  single = patterns[sorted[0]]
  if triple
    partial_score = triple
    rest = sorted[3..-1]
  elsif single
    partial_score = single
    rest = sorted[1..-1]
  else
    partial_score = 0
    rest = sorted[1..-1]
  end

  if rest
    partial_score + score(rest) 
  else
    partial_score
  end
end
deleted 112 characters in body; edited title
Source Link
Jamal
  • 34.9k
  • 13
  • 133
  • 237

Is this good Ruby? (Ruby Koans' Greed Task)

Now I'm wonderngwondering: Is this good Ruby code? If not, how would a "Rubyist" write this method? And possibly: Why? I'm also not so happy about the amount of duplicate code but can't think of a better Rubyish way.

Thanks for your reviews!

EDIT: The updated method to review (updated upon glebm's feedback)

def score(dice)
  patterns = {[1,1,1]=>1000, [2,2,2]=>200, [3,3,3]=>300, [4,4,4]=>400,
              [5,5,5]=>500,  [6,6,6]=>600, 1=>100, 5=>50}
  sorted = dice.sort

  triple = patterns[sorted[0..2]]
  single = patterns[sorted[0]]
  if triple
    partial_score = triple
    rest = sorted[3..-1]
  elsif single
    partial_score = single
    rest = sorted[1..-1]
  else
    partial_score = 0
    rest = sorted[1..-1]
  end

  if rest
    partial_score + score(rest) 
  else
    partial_score
  end
end

The original method to review

def score(dice)   #dice is an array of numbers, i.e. [3,4,5,3,3]
  return 0 if(dice == [] || dice == nil)

  dice.sort!

  return 1000 + score(dice[3..-1]) if(dice[0..2] == [1,1,1])
  return 600 + score(dice[3..-1]) if(dice[0..2] == [6,6,6])
  return 500 + score(dice[3..-1]) if(dice[0..2] == [5,5,5])
  return 400 + score(dice[3..-1]) if(dice[0..2] == [4,4,4])
  return 300 + score(dice[3..-1]) if(dice[0..2] == [3,3,3])
  return 200 + score(dice[3..-1]) if(dice[0..2] == [2,2,2])
  return 100 + score(dice[1..-1]) if(dice[0] == 1)
  return 50 + score(dice[1..-1]) if(dice[0] == 5)
  return 0 + score(dice[1..-1]);
end

**Some background (if needed)Some background (if needed)

# Greed is a dice game where you roll up to five dice to accumulate
# points. 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 fours is 400 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:


class AboutScoringProject < 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_a_single_roll_of_1_is_100
    assert_equal 200, score([1,1])
  end

  def test_score_of_multiple_1s_and_5s_is_the_sum_of_individual_scores
    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

  def test_score_of_a_triple_1_is_1000A
    assert_equal 1150, score([1,1,1,5,1])
  end

  def test_score_of_a_triple_1_is_1000B
    assert_equal 350, score([3,4,5,3,3])
  end

  def test_score_of_a_triple_1_is_1000C
    assert_equal 250, score([1,5,1,2,4])
  end
end

EDIT: The updated method

def score(dice)
  patterns = {[1,1,1]=>1000, [2,2,2]=>200, [3,3,3]=>300, [4,4,4]=>400,
              [5,5,5]=>500,  [6,6,6]=>600, 1=>100, 5=>50}
  sorted = dice.sort

  triple = patterns[sorted[0..2]]
  single = patterns[sorted[0]]
  if triple
    partial_score = triple
    rest = sorted[3..-1]
  elsif single
    partial_score = single
    rest = sorted[1..-1]
  else
    partial_score = 0
    rest = sorted[1..-1]
  end

  if rest
    partial_score + score(rest) 
  else
    partial_score
  end
end

Is this good Ruby? (Ruby Koans' Greed Task)

Now I'm wonderng: Is this good Ruby code? If not, how would a "Rubyist" write this method? And possibly: Why? I'm also not so happy about the amount of duplicate code but can't think of a better Rubyish way.

Thanks for your reviews!

EDIT: The updated method to review (updated upon glebm's feedback)

def score(dice)
  patterns = {[1,1,1]=>1000, [2,2,2]=>200, [3,3,3]=>300, [4,4,4]=>400,
              [5,5,5]=>500,  [6,6,6]=>600, 1=>100, 5=>50}
  sorted = dice.sort

  triple = patterns[sorted[0..2]]
  single = patterns[sorted[0]]
  if triple
    partial_score = triple
    rest = sorted[3..-1]
  elsif single
    partial_score = single
    rest = sorted[1..-1]
  else
    partial_score = 0
    rest = sorted[1..-1]
  end

  if rest
    partial_score + score(rest) 
  else
    partial_score
  end
end

The original method to review

def score(dice)   #dice is an array of numbers, i.e. [3,4,5,3,3]
  return 0 if(dice == [] || dice == nil)

  dice.sort!

  return 1000 + score(dice[3..-1]) if(dice[0..2] == [1,1,1])
  return 600 + score(dice[3..-1]) if(dice[0..2] == [6,6,6])
  return 500 + score(dice[3..-1]) if(dice[0..2] == [5,5,5])
  return 400 + score(dice[3..-1]) if(dice[0..2] == [4,4,4])
  return 300 + score(dice[3..-1]) if(dice[0..2] == [3,3,3])
  return 200 + score(dice[3..-1]) if(dice[0..2] == [2,2,2])
  return 100 + score(dice[1..-1]) if(dice[0] == 1)
  return 50 + score(dice[1..-1]) if(dice[0] == 5)
  return 0 + score(dice[1..-1]);
end

**Some background (if needed)

# Greed is a dice game where you roll up to five dice to accumulate
# points. 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 fours is 400 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:


class AboutScoringProject < 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_a_single_roll_of_1_is_100
    assert_equal 200, score([1,1])
  end

  def test_score_of_multiple_1s_and_5s_is_the_sum_of_individual_scores
    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

  def test_score_of_a_triple_1_is_1000A
    assert_equal 1150, score([1,1,1,5,1])
  end

  def test_score_of_a_triple_1_is_1000B
    assert_equal 350, score([3,4,5,3,3])
  end

  def test_score_of_a_triple_1_is_1000C
    assert_equal 250, score([1,5,1,2,4])
  end
end

Ruby Koans' Greed Task

Now I'm wondering: Is this good Ruby code? If not, how would a "Rubyist" write this method? And possibly: Why? I'm also not so happy about the amount of duplicate code but can't think of a better Rubyish way.

def score(dice)   #dice is an array of numbers, i.e. [3,4,5,3,3]
  return 0 if(dice == [] || dice == nil)

  dice.sort!

  return 1000 + score(dice[3..-1]) if(dice[0..2] == [1,1,1])
  return 600 + score(dice[3..-1]) if(dice[0..2] == [6,6,6])
  return 500 + score(dice[3..-1]) if(dice[0..2] == [5,5,5])
  return 400 + score(dice[3..-1]) if(dice[0..2] == [4,4,4])
  return 300 + score(dice[3..-1]) if(dice[0..2] == [3,3,3])
  return 200 + score(dice[3..-1]) if(dice[0..2] == [2,2,2])
  return 100 + score(dice[1..-1]) if(dice[0] == 1)
  return 50 + score(dice[1..-1]) if(dice[0] == 5)
  return 0 + score(dice[1..-1]);
end

Some background (if needed)

# Greed is a dice game where you roll up to five dice to accumulate
# points. 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 fours is 400 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:


class AboutScoringProject < 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_a_single_roll_of_1_is_100
    assert_equal 200, score([1,1])
  end

  def test_score_of_multiple_1s_and_5s_is_the_sum_of_individual_scores
    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

  def test_score_of_a_triple_1_is_1000A
    assert_equal 1150, score([1,1,1,5,1])
  end

  def test_score_of_a_triple_1_is_1000B
    assert_equal 350, score([3,4,5,3,3])
  end

  def test_score_of_a_triple_1_is_1000C
    assert_equal 250, score([1,5,1,2,4])
  end
end

EDIT: The updated method

def score(dice)
  patterns = {[1,1,1]=>1000, [2,2,2]=>200, [3,3,3]=>300, [4,4,4]=>400,
              [5,5,5]=>500,  [6,6,6]=>600, 1=>100, 5=>50}
  sorted = dice.sort

  triple = patterns[sorted[0..2]]
  single = patterns[sorted[0]]
  if triple
    partial_score = triple
    rest = sorted[3..-1]
  elsif single
    partial_score = single
    rest = sorted[1..-1]
  else
    partial_score = 0
    rest = sorted[1..-1]
  end

  if rest
    partial_score + score(rest) 
  else
    partial_score
  end
end
Question Protected by Jamal
Notice removed Needs detailed answers by Adam
Notice added Needs detailed answers by Adam
Tweeted twitter.com/#!/StackCodeReview/status/31681088996769794
added 690 characters in body; added 9 characters in body
Source Link
Loading
edited body
Source Link
Loading
Source Link
Loading