Class for sorting pool balls

Question

I have written this code for fun. I'd like to hear your suggestions about how to make it more compact and pythonic.

from random import shuffle

class Pool(object):
    """
    Simple object that allows to sort
    """

    def __init__(self):
        """
        During initalization the final grid containing
        ordered balls is created
        1 is placeholder for Solid balls
        0 is placeholder for Striped balls
        """
        self.grid = [[1],[0,0],[1,0,1],[0,1,0,0],[1,0,1,0,1]]

    def create_ball_set(self):
        """
        This function returns a list containing
        a shuffled pool balls set
        """
        balls = []
        [balls.append([number]) for number in range(1, 16)]
        for ball in balls:
            if ball[0] < 9:
                ball.append("Solid")
            else:
                ball.append("Striped")
        shuffle(balls)
        return balls

    def sort_ball_set(self, unsorted_balls):
        """
        This function returns a
        list of sorted balls from a list of
        shuffled balls
        """
        # Ball 1 always goes in 1st place
        self.grid[0][0] = unsorted_balls.pop(unsorted_balls.index([1, 'Solid']))
        # Ball 8 always goes in the 2nd row, in the middle
        self.grid[2][1] = unsorted_balls.pop(unsorted_balls.index([8, 'Solid']))
        # Creating an empty list for solid balls
        unsorted_solid_balls = []
        # Same thing but for striped balls
        unsorted_striped_balls = []

        # Now it is time to divide solid balls from striped ones
        for ball in unsorted_balls:
            if ball[1] == 'Solid':
                unsorted_solid_balls.append(ball)
            elif ball[1] == 'Striped':
                unsorted_striped_balls.append(ball)

        # Once the balls are divided it is time to put them in the grid
        for grid_row_index, grid_row in enumerate(self.grid):
            for grid_col_index, grid_col_value in enumerate(grid_row):
                # In their corresponding placeholder spot
                if grid_col_value == 1:
                    # 1 is for solid balls
                    self.grid[grid_row_index][grid_col_index] = unsorted_solid_balls.pop()
                elif grid_col_value == 0:
                    # 0 is for striped balls
                    self.grid[grid_row_index][grid_col_index] = unsorted_striped_balls.pop()
        return self.grid

pool_table = Pool()
print "Getting new set of balls from the pool table:"
unsorted_balls = pool_table.create_ball_set()
print unsorted_balls
print "Sorted balls:"
sorted_balls = pool_table.sort_ball_set(unsorted_balls)
# Pretty printing sorted balls
for row in sorted_balls:
    print row

Answer 1

There are several aspects of your self.grid that I don't like:

• It is constant, so it should be in ALL_CAPS.
• It can be shared by all instances of Pool, so it should be an attribute of the Pool class, not of each Pool instance.
• The convention of using 1 and 0 to represent solids and stripes is unnecessarily cryptic and necessitates comments like 1 is placeholder for Solid balls and # 1 is for solid balls. Everywhere else in your class, you are using strings 'Solid' and 'Striped', which is more obvious.
• It neglects to specify that the ① ball always goes first and the ⑧ ball always goes in the middle. These two balls then have to be handled as exceptional cases in sort_ball_set().

It's hard to say what your Pool class represents. Each instance doesn't really have any object state. (As mentioned above, self.grid doesn't count.) You're basically just using it as a kind of namespace. I would repurpose it to represent a list of balls.

---

I don't think that it is a good idea to represent each ball as a list of length 2:

• Lists are most suited for storing homogeneous items of arbitrary length. An object with a fixed number of attributes, each of a different type, would be better represented using a tuple.
• The task already necessitates dealing with two-dimensional lists. Introducing more indexing makes the code more cumbersome and less readable.
• For even better readability, you should use a namedtuple, so that you can refer to ball.n and ball.coloration instead of magic indices like ball[0] and ball[1].

---

In create_ball_set(), this is an abuse of a list comprehension solely for its side-effects:

balls = []
[balls.append([number]) for number in range(1, 16)]

The weirdness is more apparent if you run it in the REPL:

>>> balls = []
>>> [balls.append([number]) for number in range(1, 16)]
[None, None, None, None, None, None, None, None, None, None, None, None, None, None, None]
>>> balls
[[1], [2], [3], [4], [5], [6], [7], [8], [9], [10], [11], [12], [13], [14], [15]]

The conventional expression should be balls = [[n] for n in range(1, 16)].

---

Suggested solution

I would put more work into defining a TEMPLATE that fully describes what the triangle has to look like.

from collections import namedtuple
import random

PoolBall = namedtuple('PoolBall', 'n coloration')

class PoolBalls(object):
    def filter(criterion):
        return (lambda ball: ball.n == criterion) if criterion in (1, 8) else \
               (lambda ball: ball.n not in (1, 8) and ball.coloration == criterion)

    TEMPLATE = [[filter(criterion) for criterion in row] for row in [
                                  [1],
                         ['Striped', 'Striped'],
                     ['Solid',     8,      'Solid'],
               ['Striped', 'Solid', 'Striped', 'Striped'],
            ['Solid', 'Striped', 'Solid', 'Striped', 'Solid'],
        ]
    ]

    def __init__(self):
        self.balls = [PoolBall(n, 'Solid') for n in range(1, 9)] + \
                     [PoolBall(n, 'Striped') for n in range(9, 16)]

    def shuffle(self):
        random.shuffle(self.balls)

    def arrange(self):
        balls = self.balls[::-1]
        def pick(filter):
            for i in range(len(balls) - 1, -1, -1):
                if filter(balls[i]):
                    return balls.pop(i)
        return [[pick(filter) for filter in row] for row in self.TEMPLATE]

if __name__ == '__main__':
    print "Getting new set of balls from the pool table:"
    pool_table = PoolBalls()
    pool_table.shuffle()
    print pool_table.balls
    print "Arranged balls:"
    for row in pool_table.arrange():
        print row

Answer 2

I think it is usually better to take advantage of the built-in functions. Python lists already have a sort function, so you would expect a list of pool balls to be sortable by the start arrangement (a list of tuples/lists is already sortable, Python compares them element wise).

One way to achieve that is with a custom key function:

order = [1, 13, 11, 7, 8, 6, 9, 5, 14, 15, 2, 12, 3, 10, 4]
# Simplified balls list, as the fill style is not really needed for this
balls = [[n] for n in range(1, 16)]
balls.sort(key=lambda ball: order.index(ball[0]))

This is very succinct, but you could also promote the lambda key function to a proper function:

def pool_start_order(ball):
    order = [1, 13, 11, 7, 8, 6, 9, 5, 14, 15, 2, 12, 3, 10, 4]
    return order.index(ball[0])

balls = [[n] for n in range(1, 16)]
balls.sort(key=pool_start_order)

Another alternative is to make a PoolBall class, that is sortable, something like this:

class PoolBall(object):
    order = [1, 13, 11, 7, 8, 6, 9, 5, 14, 15, 2, 12, 3, 10, 4]

    def __init__(self, number):
        assert 1 <= number <= 15
        self.number = number
        self.fill = 'Solid' if number <= 8 else 'Striped'
        self.order = PoolBall.order.index(number)

    def __lt__(self, other):
        return self.order < other.order

    def __eq__(self, other):
        return self.number == other.number

    def __str__(self):
        return "{self.number} {self.fill}".format(self=self)

    def __repr__(self):
        return "{self.__class__.__name__}({self.number})".format(self=self)


    balls = [PoolBall(i) for i in range(1, 16)]
    balls.sort()

Here, the __str__ and __repr__ are just there for pretty printing.

Whichever way you go, once you have a correctly sorted list of balls, you can use a function that successively takes one more ball, something like:

def pyramid_arrangement(balls):
    """
    Given a list of balls, arranges them in a pyramid shape.
    Yields each row.

    Leaves the balls list empty.
    """
    take = 1
    while balls:
        yield [balls.pop() for _ in range(take)]
        take += 1

Which you can use like this:

if __name__ == "__main__":
    balls = [PoolBall(i) for i in range(1, 16)]
    balls.sort(reverse=True)
    for row in pyramid_arrangement(balls):
        print row