Bulls and Cows in Python

I have recently written this little Bulls and Cows game in Python:

"""Play Bulls and Cows."""
import random
NUM_DIGITS = 5

class IllegalNumber(Exception):
"""An exception to raise when the class Num has an instance of an
illegal number."""
pass

class Num(object):
"""Represent a number that consists of different NUM_DIGITS digits.
n -- a representation of the number (int, str or a
sequence of digits)."""
def __init__(self, n):
self.num = tuple(int(d) for d in str(n)) if isinstance(n, (int, str)) \
else tuple(int(d) for d in n)  # convert n into a tuple of digits
if not self._is_legal():
raise IllegalNumber

def _is_legal(self):
if len(self.num) != NUM_DIGITS:
return False
if len(set(self.num)) < NUM_DIGITS: # Does it contain equal digits?
return False
if self.num[0] == 0:
return False
if not all(0 <= d <= 9 for d in self.num):
return False
return True

def __str__(self):
return ''.join(str(d) for d in self.num)

def compare(self, other):
"""If one number was A's secret number and the second one
was B's guess, how many bulls and cows are there? Return
them in a (bulls, cows) tuple representation.

>>> Num(1234).compare(Num(5243)) ==> 1, 2
"""
bulls, cows = 0, 0
for i in xrange(NUM_DIGITS):
if self.num[i] == other.num[i]:
bulls += 1
elif self.num[i] in other.num:
cows += 1
return bulls, cows

class ComputerPlayer(object):
"""An average computer player."""
def __init__(self):
self.possible_nums = []
for num in xrange(10**(NUM_DIGITS-1), 10**NUM_DIGITS):
# Iterate over the numbers that has NUM_DIGITS digits.
try:
self.possible_nums.append(Num(num))
except IllegalNumber:
pass
self._secret_num = random.choice(self.possible_nums)

def guess(self, check_func):
"""Guess a number and check it with the given function.
The function gets a guess and returns the bulls and cows.
Return True if the player won, and False otherwise.
"""
try:
guess_num = random.choice(self.possible_nums)
except IndexError:
# If self.possible_nums is empty, the opponent cheated or
# mistaked
return True  # If one player mistakes, his opponent wins
bulls, cows = check_func(guess_num)
if bulls == NUM_DIGITS:
return True
self.possible_nums = [n for n in self.possible_nums
if n.compare(guess_num) == (bulls, cows)]
# The possible numbers that could be the opponent's secret number.
return self.possible_nums == []  # If it's empty, the opponent cheated

def check(self, guess_num):
"""Check the opponent's guess, and return the bulls and cows."""
return self._secret_num.compare(guess_num)

class HumanPlayer(object):
"""Ask the user for a guess, show him the suitable bulls and cows
for the guess, and ask him for suitable bulls and cows for the
opponent's guess - let the user play Bulls and Cows."""

def guess(self, check_func):
"""Ask the user for a guess, and check it with the
given function, that gets the guess and returns
the bulls and cows in a (bulls, cows) representation.
Print the results to the user.
Return True if the player won, and False otherwise.
"""
while True:
try:
guess_num = Num(raw_input('Enter a guess: '))
except IllegalNumber:
print ("The number should consist of %d different digits"
"and the first digit can't be 0." % NUM_DIGITS)
continue
break
bulls, cows = check_func(guess_num)
if (bulls, cows) == (NUM_DIGITS, 0):
print 'You won!'
return True
print 'You have {} bulls and {} cows.'.format(bulls, cows)
print '-----'
return False

def check(self, num):
"""Check the opponent's guess, and return the bulls and cows."""
# The messages are indented, to improve the readibility
# for the user - he can easily distinguish between messages
# from guess and messages from check
print "\tYour opponent's guess is {}.".format(num)
bulls, cows = int(raw_input('\tBulls: ')), int(raw_input('\tCows: '))
if (bulls, cows) == (NUM_DIGITS, 0):
print 'You lost!'
return bulls, cows

def play(player1, player2):
"""Play a game between the two given Player classes.
Each player class should contain guess and check methods.
check will get the opponent's guess and return the suitable
bulls and cows, and guess will get a function (the opponent's
check method) and let the player guess a number and check it
with the given function. guess will return True if the player
won, and False otherwise.
Return 1 if player1 won, and 2 if player2 won.
"""
current_player, opponent = random.sample((player1(), player2()), 2)
while not current_player.guess(opponent.check):
current_player, opponent = opponent, current_player
return {player1: 1, player2: 2}[current_player.__class__]

play(HumanPlayer, ComputerPlayer)

raw_input("Thanks for playing Bulls and Cows! ")


It works well (as far as I know), and I want it to be reviewed. What do you think about the code?

• See this answer for some more compact implementation ideas. Jan 4 '15 at 15:56

Your IllegalNumber is an exception, so should be called IllegalNumberException. It's also not needed since ValueError already occupies this niche.

You should break this onto multiple lines:

self.num = tuple(int(d) for d in str(n)) if isinstance(n, (int, str)) \
else tuple(int(d) for d in n)  # convert n into a tuple of digits


as

if isinstance(n, (int, str)):
self.num = tuple(int(d) for d in str(n))
else:
self.num = tuple(int(d) for d in n)


although it's simpler to do:

if isinstance(n, (int, str)):
n = str(n)

self.num = tuple(map(int, n))


In fact, the isinstance check doesn't need to include str.

I would change _is_legal to

digits = set(range(10))

return self.num[0] and len(self.num) == NUM_DIGITS == len(digits & set(self.num))


I would personally use map in __str__.

compare can use zip:

bulls, cows = 0, 0
for digit, other_digit in zip(self.num, other.num):
if digit == other_digit:
bulls += 1
elif digit in other.num:
cows += 1
return bulls, cows


You could even do this:

pos_eq = sum(map(eq, self.num, other.num))
set_eq = len(set(self.num) & set(other.num))

return pos_eq, set_eq - pos_eq


although maybe I'm getting ahead of myself.

ComputerPlayer.__init__ can generate mostly correct digits with:

for digits in permutations(range(10), NUM_DIGITS)


This gives a good speed improvement to the initial load. Since the load is still noticably slow, I suggest improving it with something like

    digitss = permutations(range(10), NUM_DIGITS)

for digits in digitss:
if digits[0]:
break

self.possible_nums = [digits] + map(Num, digitss)


and making the _is_legal call external so that this loop doesn't require it. I would also change the constructor to just

self.num = n


since permutations already returns tuples of digits. This would make HumanPlayer.guess start with

while True:
inp = raw_input('Enter a guess: ')

try:
digits = map(int, inp)
except ValueError:
pass
else:
guess_num = Num(digits)
if guess_num.is_legal():
break

print ("The number should consist of %d different digits "
"and the first digit can't be 0." % NUM_DIGITS)


In ComputerPlayer.guess, you shouldn't need two checks to determine if the human cheated. Namely, the first try...except seems unneeded.

I like the use of a higher-order function with check_func. It's very elegant. However, don't abuse it; HumanPlayer should not be running the game; that logic should be in play.

It's unfortunate that you don't get an indication that you lost, or if you lost due to being caught cheating. This would be easier to sort out if more was in play and not in the respective classes.

You only use each method of Num in one place with these changes; it makes sense IMHO to make these functions on tuples of digits to remove the need to create classes. This makes things a little cleaner. It also allows one to speed up ComputerPlayer.__init__ further.

You don't validate the user's input in HumanPlayer.check. check also shouldn't be in the business of saying You lost!; that should be game logic.

Using random.sample to shuffle in-place is fancy but it isn't good. Just use random.shuffle on another line.

This all gives:

"""Play Bulls and Cows."""
import random
NUM_DIGITS = 5

from itertools import permutations
from operator import eq

def is_legal(digits):
return digits[0] and len(digits) == NUM_DIGITS == len(set(range(10)) & set(digits))

def format_digits(digits):
return ''.join(map(str, digits))

def bulls_and_cows(left, right):
"""If one number was A's secret number and the second one
was B's guess, how many bulls and cows are there? Return
them in a (bulls, cows) tuple representation.

>>> bulls_and_cows((1, 2, 3, 4), (5, 2, 4, 3)) ==> 1, 2
"""
pos_eq = sum(map(eq, left, right))
set_eq = len(set(left) & set(right))

return pos_eq, set_eq - pos_eq

def get_int(inp):
while True:
try:
return int(raw_input(inp))
except ValueError:
pass

class ComputerPlayer(object):
"""An average computer player."""
def __init__(self):
self.possible_nums = list(permutations(range(10), NUM_DIGITS))

minimum = self.possible_nums.index(tuple(range(1, NUM_DIGITS+1)))
del self.possible_nums[:minimum]

self._secret_num = random.choice(self.possible_nums)

def guess(self, check_func):
"""Guess a number and check it with the given function.
The function gets a guess and returns the bulls and cows.
Return True if the player won, and False otherwise.
"""
guess_num = random.choice(self.possible_nums)
bulls, cows = check_func(guess_num)

if bulls == NUM_DIGITS:
return True

# The possible numbers that could be the opponent's secret number.
self.possible_nums = [n for n in self.possible_nums if bulls_and_cows(n, guess_num) == (bulls, cows)]

# If it's empty, the opponent cheated
return self.possible_nums == []

def check(self, guess_num):
"""Check the opponent's guess, and return the bulls and cows."""
return bulls_and_cows(self._secret_num, guess_num)

class HumanPlayer(object):
"""Ask the user for a guess, show him the suitable bulls and cows
for the guess, and ask him for suitable bulls and cows for the
opponent's guess - let the user play Bulls and Cows."""

def guess(self, check_func):
"""Ask the user for a guess, and check it with the
given function, that gets the guess and returns
the bulls and cows in a (bulls, cows) representation.
Print the results to the user.
Return True if the player won, and False otherwise.
"""
while True:
inp = raw_input('Enter a guess: ')

try:
guess_num = map(int, inp)
except ValueError:
pass
else:
if is_legal(guess_num):
break

print ("The number should consist of %d different digits "
"and the first digit can't be 0." % NUM_DIGITS)

bulls, cows = check_func(guess_num)

if (bulls, cows) == (NUM_DIGITS, 0):
print 'You won!'
return True

print 'You have {} bulls and {} cows.'.format(bulls, cows)
print '-----'
return False

def check(self, num):
"""Check the opponent's guess, and return the bulls and cows."""
# The messages are indented, to improve the readibility
# for the user - he can easily distinguish between messages
# from guess and messages from check
print "\tYour opponent's guess is {}.".format(format_digits(num))
bulls, cows = get_int('\tBulls: '), get_int('\tCows: ')

if (bulls, cows) == (NUM_DIGITS, 0):
print 'You lost!'

return bulls, cows

def play(player1, player2):
"""Play a game between the two given Player classes.
Each player class should contain guess and check methods.
check will get the opponent's guess and return the suitable
bulls and cows, and guess will get a function (the opponent's
check method) and let the player guess a number and check it
with the given function. guess will return True if the player
won, and False otherwise.
Return 1 if player1 won, and 2 if player2 won.
"""
players = [player1(), player2()]
random.shuffle(players)

current_player, opponent = players
while not current_player.guess(opponent.check):
current_player, opponent = opponent, current_player

return {player1: 1, player2: 2}[current_player.__class__]

play(HumanPlayer, ComputerPlayer)

raw_input("Thanks for playing Bulls and Cows! ")

• Thanks for the review! However, HumanPlayer.guess and HumanPlayer.check still print You lost! and You won! in the final code - you didn't change that. I think the play function has to call a HumanPlayer.end_of_game (along with ComputerPlayer.end_of_game) method (with a boolean parameter for win/lose) that will print the messages - ComputerPlayer.end_of_game will just pass. What do you think? Jan 5 '15 at 17:57
• @Avnerium "you didn't change that" → Yes, I didn't really want to make even larger changes myself. Your solution seems great: +1. Jan 9 '15 at 11:46
• It look like a common behaviour you want a Player to have is the ability of def check(self, num): and def guess(self, check_func):. Consider adding an ABC to this code class Player with the 2 @abstractmethods and have HumanPlayer and ComputerPlayer inherit from this.

• Move all the try and expect logic into a separate method (consider moving it in the class Player from the comment above if you decide to go that way), and wrap all you raw_input calls with this method. The idea being keep looking until you get a valid input from the user (youre' skipping this logic in bulls, cows = int(raw_input('\tBulls: ')), int(raw_input('\tCows: ')) .. which will just crash with invalid input)

Larger Note : I've been thinking the code in 5 sections (maybe reflecting hte code in these sections might make it more readable?):

1. NumberValidation: the class solely responsible for running all the raw_input and validating the input until we get one (no one outside of this should be calling raw_input or running validation on the input values .. the game is "stuck" at this until a valid input is recognized)
2. GameValidation: the class responsible for checking the state of the game and calling it complete once we have all bulls (a lot of the compare and some of the guess functionality can go in here)
3. Player: the ABC class that represents what a player should look like with the proper default behavior
4. The HumanPlayer and the ComputePlayer class implementations
5. The main method for game play between HumanPlayer and the ComputerPlayer.

You could remove duplicates logic in

self.num = tuple(int(d) for d in str(n)) if isinstance(n, (int, str)) else tuple(int(d) for d in n)


By writing :

self.num = tuple(int(d) for d in (str(n) if isinstance(n, (int, str)) else n))