2
\$\begingroup\$

This craps game simulator is based on someone else's posted Python code that I've redone for practice. I'm interested in improving readability and making it more pythonic. I used unittest instead of nose because I'm using an online IDE.

from random import randrange
import unittest

class CrapsGame:
    def __init__(self):
        self.outcomes = {'2':False,'3':False,'12':False,'7':True,'11':True,
                '4&4':True,'5&5':True,'6&6':True,
                '8&8':True,'9&9':True,'10&10':True,
                '4&7':False,'5&7':False,'6&7':False,
                '8&7':False,'9&7':False,'10&7':False}

    def play(self, roll_dice):
        comeOut = str(roll_dice())
        print 'began game with ' + comeOut
        if comeOut in self.outcomes:
            return self.outcomes[comeOut]
        while True:
            point = str(roll_dice())
            print 'next roll is ' + point
            state = comeOut+'&'+point
            if state in self.outcomes:
                return self.outcomes[state]


class CrapsTest(unittest.TestCase):
    def testWinComeOut(self):
        game = CrapsGame()
        self.assertEquals(game.play(lambda: 7), True)

    def testLoseComeOut(self):
        game = CrapsGame()
        self.assertEquals(game.play(lambda: 2), False)

    def testWinPoint(self):
        game = CrapsGame()
        rollData = [5,6,5]
        self.assertEquals(game.play(lambda: rollData.pop()), True)

    def testLosePoint(self):
        game = CrapsGame()
        rollData = [7,5]
        self.assertEquals(game.play(lambda: rollData.pop()), False)

if __name__ == '__main__':
    unittest.main()
\$\endgroup\$
1
  • \$\begingroup\$ I can give my 2 cents in regard to the naming conventions. Using snake case for the variables' names would be definitely more pythonic, for example come_out instead of comeOut. \$\endgroup\$
    – cezar
    Feb 27, 2020 at 17:37

1 Answer 1

3
\$\begingroup\$

Some general comments first.

  1. There's no need to make your throws in strings for the outcomes - you can use tuples instead. e.g.

    self.outcomes = { (2,):False, (5,5):True }
    
  2. If you pass a "wrong" set of dice throws (say \$[4,5]\$), you'll have an exception raised which isn't dealt with (and should probably be a test?).

  3. pop removes the last element, which means you process them in reverse order - which differs from what the casual reader might expect (\$[7,5]\$ = 7 first, then 5).

You may want to look at generators which would provide a nice interface to a throw. In this case, what about:

class DiceThrow:
    def __init__( self, rolls = None ):
        self.rolls = rolls
    def First( self ):
        return self.__next__()
    def __iter__( self ):
        return self
    def __next__( self ):
        if self.rolls is not None:
            if len( self.rolls ) == 0:
                raise StopIteration
            r = self.rolls[0]
            self.rolls = self.rolls[1:]
            return r
        return randrange( 1, 13 ) # may be better as randint( 1, 12 )

This can then be called as follows:

game.play( DiceThrow( [5,7] ) )
game.play( DiceThrow() ) # for a random set of throws

and used:

def play(self, dice_throw):
    comeOut = dice_throw.First()
    print( "began game with %d"%comeOut )
    if (comeOut,) in self.outcomes:
        return self.outcomes[(comeOut,)]
    for point in dice_throw:
        print( "next roll is %d"%point )
        state = (comeOut,point)
        if state in self.outcomes:
            return self.outcomes[state]
    print( "Bad rolls!" )
\$\endgroup\$
1
  • \$\begingroup\$ Thank you. I appreciate the effort. I like the tuples instead of strings, and the idea of using a generator sounds interesting. \$\endgroup\$
    – jlim
    Jul 25, 2012 at 6:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.