Python: die rolling for ordering

Question

I've little experience with python and am trying to become more familiar with it. As an exercise, I 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 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()

Answer 1

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)]

Answer 2

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]