Die-rolling for ordering

I have a little experience with Python and am trying to become more familiar with it. As an exercise, I've tried coding a program where the ordering of a list of players is determined by the roll of a die. A higher number would place the player ahead of others with lower numbers. Ties would be broken by the tied players rolling again.

Here is an example:

                           player 1, player 2, player 3
rolls 6, rolls 6, rolls 1
/           \
/             \
player 1, player 2    player 3
rolls 5, rolls 4
/       \
/         \
player 1    player 2


Final order: player 1, player 2, player 3

I'm interested in how I could have done this better. In particular, I'm not familiar with much of the Python style guide so making it more pythonic would be nice. Also, I'm not that familiar with unit testing. I've used unittest instead of nose since I was using an online IDE that supports it.

from random import randint
from itertools import groupby
import unittest

def rank_players(playerList, die):
tree = [playerList]
nextGeneration = []
keepPlaying = True
while keepPlaying:
keepPlaying = False
for node in tree:
if len(node) == 1:
nextGeneration.append(node)
else:
keepPlaying  = True
rolls = [die.roll() for i in range(len(node))]
turn = sorted(zip(rolls,node), reverse=True)
print 'players roll:', turn
for key, group in groupby(turn, lambda x: x[0]):
nextGeneration.append(list(i[1] for i in group))
tree = nextGeneration
nextGeneration = []
return [item for sublist in tree for item in sublist]

class Die:
def __init__(self,rolls=[]):
self.i = -1
self.rolls = rolls

def roll(self):
self.i = self.i + 1
return self.rolls[self.i]

class RankingTest(unittest.TestCase):

def testEmpty(self):
die = Die()
players = []
self.assertEquals(rank_players(players,die),[])

def testOnePlayer(self):
die = Die()
player = ['player 1']
self.assertEquals(rank_players(player,die),['player 1'])

def testTwoPlayer(self):
die = Die([6, 1])
players = ['player 1', 'player 2']
self.assertEquals(rank_players(players,die),['player 1', 'player 2'])

def testThreePlayer(self):
die = Die([6, 6, 1, 4, 5])
players = ['player x', 'player y', 'player z']
self.assertEquals(rank_players(players,die),['player x', 'player y','player z'])

def testRunAll(self):
self.testEmpty()
self.testOnePlayer()
self.testTwoPlayer()
self.testThreePlayer()

if __name__ == '__main__':
unittest.main()

• Is there a reason for calling a dice "die"? BTW you can find some Python style guides here Jul 23 '12 at 10:01
• Take also a look here to know why you shouldn't do def __init__(self,rolls=[]): Jul 23 '12 at 10:07
• Thanks for the links. I used the singular for dice, die, in an attempt to convey that each player would only do one roll for each turn. I would've liked a less ambiguous term.
– jlim
Jul 23 '12 at 15:46

Here's one way it could be done with a lot less code:

from itertools import groupby
from operator import itemgetter
from random import randint

def rank_players(playerList):
playerRolls = [(player, randint(1, 6)) for player in playerList]
playerGroups = groupby(
sorted(playerRolls, key=itemgetter(1), reverse=True),
itemgetter(1))
for key, group in playerGroups:
grouped_players = list(group)
if len(grouped_players) > 1:
for player in rank_players(zip(*grouped_players)[0]):
yield player
else:
yield grouped_players[0]


Usage:

>>> print list(rank_players(["bob", "fred", "george"]))
[('fred', 5), ('george', 6), ('bob', 4)]

• I really appreciate your answer. I will look more carefully into it at the end of today. My impression is though, that this answer does not handle the case of two ties the way I was hoping to. For example, player 1 and player 2 tie with 5, player 3 is in the middle with 4, and player 4 and player 5 tie with 3. Thank you for your effort. I'm not sure of the procedure, but I will probably mark it as the answer by the end of today if it is the best available answer.
– jlim
Jul 23 '12 at 15:52
• @jlim: Ah, I see (hence your use of groupby.) Well this solution is easily extensible to that end, since the recursion works on any set of sublists of players. I am a little preoccupied at the moment, but I can probably post a modified solution in a couple of hours. Jul 23 '12 at 17:14
• @jlim: Updated my answer. This version will handle the possiblity of n ties, provided that n does not exceed the recursion limit ;) Jul 23 '12 at 19:36
• Thank you for the update. Sorry I don't have enough rep to up vote yet.
– jlim
Jul 24 '12 at 1:33
• @jlim: No problem man. Glad to help Jul 24 '12 at 2:02
from random import randint
from itertools import groupby
import unittest

def rank_players(playerList, die):


Python convention is to use names like player_list for arguments/local variables

    tree = [playerList]


This isn't really used to hold a tree. Its only ever two levels deep, so its a list of lists. You also shouldn't name variables after datastructures, you should name after the meaning in the code.

    nextGeneration = []


You should move this into the loop. That way you won't have to repeat it later

    keepPlaying = True
while keepPlaying:
keepPlaying = False


Its best to try and avoid boolean logic flags. They tend to make code harder to follow. If you can, try to structure code to avoid their use.

        for node in tree:
if len(node) == 1:
nextGeneration.append(node)
else:
keepPlaying  = True
rolls = [die.roll() for i in range(len(node))]


I'd have done:

  rolls = [ (die.role, item) for item in node]


This avoids the pointless call to len/range, and also handles the zipping in the next line

                turn = sorted(zip(rolls,node), reverse=True)
print 'players roll:', turn
for key, group in groupby(turn, lambda x: x[0]):
nextGeneration.append(list(i[1] for i in group))


Joel's recursive solution is much nicer, I think.

        tree = nextGeneration
nextGeneration = []
return [item for sublist in tree for item in sublist]

class Die:
def __init__(self,rolls=[]):


In general, don't assign mutable objects as defaults. Here I don't even see why you would have a default since the object is useless with the default constructor.

        self.i = -1


I'd use index, just to be more explicit

        self.rolls = rolls

def roll(self):
self.i = self.i + 1
return self.rolls[self.i]


But here's how I'd write your function

def rank_players(player_list)
random.shuffle(player_list)


All you are doing in the end is shuffling the list of players, and python has a function for that. Unless you really want to simulate dice, there isn't a lot of point in doing it.

Another approach, I'm not sure a good one, would be:

class Die(object):
def __init__(self, player):
self.player = player
self.rolls = []

def __getitem__(self, index):
# if you don't provide an __iter__
# python will use __getitem__

# whenever an attempt is made to access
# past the end, we just roll a new number
if index == len(self.rolls):
roll = random.randrange(6)
print "Player: %s Rolls a: %d" % (self.player, roll)
self.rolls.append(roll)
return roll
else:
return self.rolls[index]

def __cmp__(self, other):
# the only Die I am equal to is myself
if self is other:
return 0

# look at the rolls in both die to
# find first one that's different
for a, b in itertools.izip(self, other):
if a != b:
return cmp(a,b)

assert False

def rank_players(player_list):
decorated = [(Die(player), player) for player in player_list]
decorated.sort(reverse = True)
print [die.rolls for die, player in decorated]
return [player for die, player in decorated]

• Thank you for your input. I agree shuffle would be better, but I wanted to try out the dice rolling and node expansion idea. Sorry, I don't have enough rep to upvote yet. I will look over the rest of your answer later.
– jlim
Jul 24 '12 at 1:42