HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Scoring solution for Farkle (dice game)?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
scoringfarklegamediceforsolution

Problem

I've written the following function on Python (3.3) to take a list of 6 integers and return the maximum score for a game of Farkle. While the results are correct I don't feel it's a very Pythonic implementation. I'd like to hear from the community on how I could make it better and more Pythonic. Any and all tips are most welcome!

```
"""
written and tested in Python 3.3.6
"""

from random import randint

def score(my_roll, sides=6):
"""
Calculate a Farkle score using the traditional scoring method, which is:
4 1s: 2,000 points
3 1s: 1,000 points
Single 1: 100 points
Single 5: 50 points
Triple of any non-1 number: 100 x number showing
Quadruple of any non-1 number: Double the triple score

Notes:
- Doubles score nothing (unless a 1 or a 5) as above
- All scoring dice must be rolled in a single turn (i.e. they
are not additive over turns)
- Rolling all 6 will be two sets and scored accordingly
[4] + [2]
[3] + [3]
[2] + [2] + [2]

Examples:
1,1,1,5,5,5 ==> 1500 (1000 for 1s + 250 for 3 5s)
1,1,1,1,6,6 ==> 2000 (2000 for 4 1s)
5,3,6,5,3,3 ==> 400 (300 for 3 3s + 100 for 2 5s)
1,2,2,3,3,5 ==> 150 (100 for a 1 and 50 for a 5)
"""
#create a table to hold the count of each die roll
dice_array = [0] * sides
score = 0

#add up the number appearances of each die roll and store it in the table
for dice in my_roll:
dice_array[dice-1] += 1

"""
based on the above scoring, determine the MAXIMUM score; in actual Farkle the
player would choose which die to 'bank' and which to re-roll
"""
for (i, count) in enumerate(dice_array):
dice = i + 1 #this makes it easier to keep track of the die we're on

if dice == 1:
if count == 6: score += 2200

Solution

"Pythonic" programming

Code is "pythonic" when it expresses its intention clearly, is easy to read or even looks like pseudo-code, and uses as little low-level garbage as possible. Let's take a look at a couple examples of low-level work in your code:

dice_array = [0] * sides
score = 0
for dice in my_roll:
    dice_array[dice-1] += 1
# as an aside, this isn't an array, it's a list.


This is wonky because it buids a fixed-size list, then plays with values-as-list-indexes to get a count. That might be fast, but it's certainly ugly. Let's not do that.

import collections

counts = collections.Counter(my_roll)


This is much better. It builds a dict-like object, a collections.Counter that has the counts for any dice present in the roll. counts[non_rolled_die] will throw a KeyError while counts.get(non_rolled_die, 0) will act like your current code does.

Your counting loop isn't a whole lot better. Let's look:

for i, count in enumerate(dice_array):
    dice = i + 1


We'll stop here for just long enough to mention that enumerate takes a keyword argument start that lets you tell it where to start counting from. This should be:

for die, count in enumerate(dice_array, start=1):


But since I switched to using a collections.Counter, we can just do:

for die, count in counts.items():
    if die == 1:
        if count == 6: score += 2200
        if count == 5: score += 2100
        if count == 4: score += 2000
        if count == 3: score += 1000
        if count in [1, 2]: score += (count * 100)
    else:
        if count >= 4: score += (dice * 200)
        if count >= 3: score += (dice * 100)
        if (die == 5 and count != 3): score += (count * 50)


First of all, avoid these repeated if statements. Try instead:

if die == 1:
        if count == 3: score += 1000
        elif count == 4: score += 2000
        elif count > 4:
            score += 2000 + (count - 4) * 100


However there's a better approach. We already know that the triple value for each number is 100 number, except for 1 which is 100 10 * number. Let's use that!

for die, count in counts.items():
    die_score = 0
    if count == 4:
        die_score += die * 2000 if die == 1 else die * 200
        count -= 4
    elif count == 3:
        die_score += die * 1000 if die == 1 else die * 100
        count -= 3


This should handle triplets and quadruplets of any value, and removes from count the number of dice consumed by the grouping. Now let's look at single dice values.

if die in [1, 5]:
        single_value = 100 if die==1 else 50
        die_score += single_value * count


In fact we can factor some of that out to drop the if block.

# somewhere earlier in the function
    single_values = {1: 100, 5:50}

    # then inside the loop we're looking at here....
    die_score += single_values.get(die, 0) * count


Finally at the end of our loop (just before exiting it), we add the single die_score to the roll-wide score.

score += die_score


Testing

I'm a big fan of the unittest module for writing tests. In your case doctest may work better (since you've written examples already that only have to be tweaked slightly to use doctest), but if you start writing more intense functions, it's important to be able to grow your testing suite appropriately. Let's write some tests!

# ./test_farkle.py

import unittest
import farkle

class FarkleTests(unittest.TestCase):
    cases = [([1,1,1,5,5,5], 1500),
             ([1,1,1,1,6,6], 2000),
             ([5,3,6,5,3,3], 400),
             ([1,2,2,3,3,5], 150)]

    def testRolls(self):
        for got, want in self.cases:
            self.assertEqual(farkle.score(got), want)
            # fails test if `farkle.score(got) != want`


A change in data structure

It should occur to you that for every (die, count) pair, score increases by a given amount. This means we can hardcode that data in something like a dict.

# {die: {count: value, ... }, ... }
valuedict = {1: {1: 100,
                 2: 200,
                 3: 1000,
                 4: 2000,
                 5: 2100,
                 6: 2200},
             2: {3: 200,
                 4: 400},
             3: {3: 300,
                 4: 600},
             4: {3: 400,
                 4: 800},
             5: {1: 50,
                 2: 100,
                 3: 500,
                 4: 1000,
                 5: 1050,
                 6: 1100},
             6: {3: 600,
                 4: 1200}}


Now your function becomes pretty simple!

def score(my_roll, sides=6):
    valuedict = ...  # the whole deal above
    counts = collections.Counter(my_roll)
    score = 0
    for die, count in counts.items():
        score += valuedict[die][count]
    return score


Or even more simply:

```
def score(my_roll, sides=6):
valuedict = ...
counts = collections.Counter(my_roll)
return sum(valuedict[die][count] for die,co

Code Snippets

dice_array = [0] * sides
score = 0
for dice in my_roll:
    dice_array[dice-1] += 1
# as an aside, this isn't an array, it's a list.
import collections

counts = collections.Counter(my_roll)
for i, count in enumerate(dice_array):
    dice = i + 1
for die, count in enumerate(dice_array, start=1):
for die, count in counts.items():
    if die == 1:
        if count == 6: score += 2200
        if count == 5: score += 2100
        if count == 4: score += 2000
        if count == 3: score += 1000
        if count in [1, 2]: score += (count * 100)
    else:
        if count >= 4: score += (dice * 200)
        if count >= 3: score += (dice * 100)
        if (die == 5 and count != 3): score += (count * 50)

Context

StackExchange Code Review Q#116051, answer score: 8

Revisions (0)

No revisions yet.