patternpythonMinor

Pig dice game with a human and a computer player

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

humanwithplayergamedicecomputerandpig

Problem

To help my son with his programming skills, I am trying refresh my limited Python knowledge. I chose a simple dice game called Pig Dice.

```
import sys
import random

HUMAN = 0
COMPUTER = 1
HOLD = False
ROLL = True

class Die:
def __init__(self):
self.value = random.randint(1, 6)

def roll(self):
self.value = random.randint(1, 6)

@staticmethod
def rolled_one():
print("Rolled 1. \n")

def __str__(self):
return "Rolled " + str(self.value) + "."

class Box:
def __init__(self):
self.value = 0

def reset_box(self):
self.value = 0

def add_dice_value(self, dice_value):
self.value = self.value + dice_value

def __str__(self):
return "Box total:" + str(self.value)

class Player:
def __init__(self, name=None):
self.name = name
self.score = 0

def add_score(self, player_score):
self.score = self.score + player_score

def set_name(self, name):
self.name = name

def __str__(self):
return str(self.name) + ": " + str(self.score)

class ComputerPlayer(Player):
def __init__(self):
Player.__init__(self, name="ELIZA")

@staticmethod
def make_decision(box):
while box.value " + str(self.human) + "\t" + str(self.computer) + "\n"

class GameManager:
def __init__(self):
self.computer_player = ComputerPlayer()
self.human_player = HumanPlayer()
self.die = Die()
self.box = Box()
self.score = Score(self.human_player, self.computer_player, self.box)
self.computer = 0
self.turn = None
self.action = None

@staticmethod
def welcome():
print("" 70)
print("Welcome to Pig Dice!" .center(70))
print("" 70)
print("The objective is to be the first to reach 100 points." .center(70))
print("On each turn, the computer will roll a die." .center(70))
print("The die value will stored in a tempora

Solution

Style review of current code

Reading up on PEP8 is never a bad thing. Mostly your code is clean, but there are several enhancements which can be made.

Add more space between methods – Methods (and functions) according to guidelines should have two new lines before them. This helps separating the methods from each other

Methods and classes should have docstrings – All methods and classes should have docstrings describing why they exist.

You mostly have good names – Most variables, classes and methods have names according to standard. The only deviation could arguably be that internal/private methods should be named _snake_case() something.

I'm not too fond of the welcome() print formatting, but this is a minor point, and in my code refactor I've just left it as is. You could have used multi-line texts, or lists of texts, and various other options.

Verify/validate input – When asking for input, it is often useful to verify that the user entered legal options. This can be done using an external method

Version compatible calling of parent constructor – I'm not sure if you're using Python 3 or Python 2, but a way to call the parent constructor in both version are to use super(HumanPlayer, self).__init__(name) where you replace the current class name instead of HumanPlayer.

Use var += 1 to increment variables – This looks a little neater than doubling the variable name

Code and class review

Previous was global comments, but here are some more localised to your choices of classes and code structure:

Drop the Score class – Let each player handle its own score, and then you can drop this entirely

Allow multiple players through a list of players – If you instead of two locked player variables, uses a list of players, you can pass how many human players and how many computer players to the constructor.

Let the turn / current player be index into player list – Instead of duplicating code, for the specialised player variables, you can let the current player be an index into the player list. This will simplify logic quite a bit. You can then start utilising the object structure, by simply adding scores to the current player, get the name of current player, and so on directly. You could take it all the way, and have a dedicated variable which holds the current player, but I chose to use the index

To change turns, add with modulo – To change turns, you can add one to the current player index, and take the modulo of number of players.

-
Try avoid one line methods – In most cases have a one line method makes it harder to understand code segments as you need to jump back and forth to see what that method actually did. If the code is simple and rather intuitive, avoid the one line method. If the method is needed as you use it in contexts like filtering, mapping or similar, they could be of use (but then again you can also use lambda's).

This is a balance, where you need to balance between readability and maintainability. No black and white answers exists here, and you need to go a little bit by gut feeling, and not everyone will agree with choices we make

If possible avoid globals – In your case to define HOLD = False and ROLL = True. With good method names, it is much more intuitive to use the original values. Let keep_rolling() return True or False directly. The two others are eliminated as we now uses proper names for each player, and use the player list for access.

Alternative handling of rolling 1 – Instead of your logic, I opted for raising an exception when rolling 1. This can be a matter of taste, but I kind of felt that it a nice flow to it to use a raised RolledOneException.

Avoid set_variable(), when you keep it public anyway – It looked kind of strange to have Player.set_name(), when you can do Player.name = ... without violating guidelines. If you had chosen to have Player._name, and possibly had other side effects needed to be executed at the same time, you could have a set_name(), but as is was I thought it looked a little strange. I neither saw the need for all the __str__ you had defined, but your mileage may vary...

The if __name__ == '__main__': pattern – Adding this pattern before calling the main() allows for your code to be runned directly as a script, but also to be easily included as a module. I've used it the code to ask for how many humans and computers are to play, and could easily have read that from the arguments as well. And still, if used as a module, you could initiate the game from another script if you felt like it.

Code refactored

Here is the code when all of the above has been taken into account:

```
import random

def input_number(prompt='Please enter a number: ', minimum=0, maximum=None):
"""Read a positive number with the given prompt."""

while True:
try:
number = int(input(prompt))
if (number maximum)):
print('Number is not with

Code Snippets

import random


def input_number(prompt='Please enter a number: ', minimum=0, maximum=None):
    """Read a positive number with the given prompt."""

    while True:
        try:
            number = int(input(prompt))
            if (number < minimum or
                (maximum is not None and number > maximum)):
                    print('Number is not within range: {} to {}'.format(minimum, maximum))
            else:
                break

        except ValueError:
            print('You need to enter a number')
            continue

    return number


class RolledOneException(Exception):
    pass


class Die:
    """A die to play with."""

    def __init__(self):
        self.value = random.randint(1, 6)

    def roll(self):
        """Returns the rolled dice, or raises RolledOneException if 1."""

        self.value = random.randint(1, 6)
        if self.value == 1:
            raise RolledOneException

        return self.value


    def __str__(self):
        return "Rolled " + str(self.value) + "."


class Box:
    """Temporary score box holder class."""

    def __init__(self):
        self.value = 0


    def reset(self):
        self.value = 0


    def add_dice_value(self, dice_value):
        self.value += dice_value


class Player(object):
    """Base class for different player types."""

    def __init__(self, name=None):
        self.name = name
        self.score = 0


    def add_score(self, player_score):
        """Adds player_score to total score."""

        self.score += player_score


    def __str__(self):
        """Returns player name and current score."""

        return str(self.name) + ": " + str(self.score)


class ComputerPlayer(Player):
    cpu_names=['Eliza', 'BigBlue', 'Foo', 'Bar']


    def __init__(self, number):
        """Assigns a cpu name from cpu_names, or Cpu#."""

        if number < len(self.cpu_names):
            name = self.cpu_names[number]
        else:
            name = 'Cpu{}'.format(number)

        super(ComputerPlayer, self).__init__(name)


    def keep_rolling(self, box):
        """Randomly decides if the CPU player will keep rolling."""

        while box.value < (10 + random.randint(1, 35)):
            print("  CPU will roll again.")
            return True
        print("  CPU will hold.")
        return False


class HumanPlayer(Player):
    def __init__(self, name):
        super(HumanPlayer, self).__init__(name)


    def keep_rolling(self, box):
        """Asks the human player, if they want to keep rolling."""

        human_decision = input_number("  1 - Roll again, 0 - Hold? ", 0, 1)
        if human_decision == 1:
            return True
        else:
            return False


class GameManager:
    def __init__(self, humans=1, computers=1):
        """Initialises the game, optionally asking for human player names."""

        self.players = []
        if humans == 1:
            self.players.append(HumanPlayer('Human'))
        else:
            for i in range(humans):

Context

StackExchange Code Review Q#110965, answer score: 5

Revisions (0)

No revisions yet.