Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Here's my Python implementation of Conway's Game of Life:

class Game(object):

    def __init__(self, state, infinite_board = True):

        self.state = state
        self.width = state.width
        self.height = state.height
        self.infinite_board = infinite_board

    def step(self, count = 1):

        for generation in range(count):

            new_board = [[False] * self.width for row in range(self.height)]

            for y, row in enumerate(self.state.board):
                for x, cell in enumerate(row):
                    neighbours = self.neighbours(x, y)
                    previous_state = self.state.board[y][x]
                    should_live = neighbours == 3 or (neighbours == 2 and previous_state == True)
                    new_board[y][x] = should_live

            self.state.board = new_board

    def neighbours(self, x, y):

        count = 0

        for hor in [-1, 0, 1]:
            for ver in [-1, 0, 1]:
                if not hor == ver == 0 and (self.infinite_board == True or (0 <= x + hor < self.width and 0 <= y + ver < self.height)):
                    count += self.state.board[(y + ver) % self.height][(x + hor) % self.width]

        return count

    def display(self):
        return self.state.display()

class State(object):

    def __init__(self, positions, x, y, width, height):

        active_cells = []

        for y, row in enumerate(positions.splitlines()):
            for x, cell in enumerate(row.strip()):
                if cell == 'o':
                    active_cells.append((x,y))

        board = [[False] * width for row in range(height)]

        for cell in active_cells:
            board[cell[1] + y][cell[0] + x] = True

        self.board = board
        self.width = width
        self.height = height

    def display(self):

        output = ''

        for y, row in enumerate(self.board):
            for x, cell in enumerate(row):
                if self.board[y][x]:
                    output += ' o'
                else:
                    output += ' .'
            output += '\n'

        return output

glider = """ oo.
             o.o
             o.. """

my_game = Game(State(glider, x = 2, y = 3, width = 10, height = 10))
print my_game.display()
my_game.step(27)
print my_game.display()

Output:

 . . . . . . . . . .
 . . . . . . . . . .
 . . o o . . . . . .
 . . o . o . . . . .
 . . o . . . . . . .
 . . . . . . . . . .
 . . . . . . . . . .
 . . . . . . . . . .
 . . . . . . . . . .
 . . . . . . . . . .

 . . . . . . . . . .
 . . . . . . . . . .
 . . . . . . . . . .
 . . . . . . . . . .
 . . . . . . . . . .
 . . . . . . o . . .
 . . . . . o o . . .
 . . . . . o . o . .
 . . . . . . . . . .
 . . . . . . . . . .

I have several concerns:

  1. It feels a bit unnatural to write self.state.board[y][x], instead of [x][y]. However, I thought it would make sense to let the board be an array of rows rather than columns.

  2. I'm not sure how to divide the tasks between the two classes. For instance, I could have implemented the neighbours() function in the State class, rather than in the Game class.

  3. I know it's good practice to write a lot of comments, but I found most of this code self-explanatory (though that could be because I've just written it).

  4. I used several nested for-loops, but maybe I could replace some with list comprehensions.

  5. I could have left out the display() function of the Game class and just write print my_game.state.display() on the last line. Does this simplify things, or does it only make it more complicated?

This is the first real thing I've written in Python, any feedback is appreciated. :)

share|improve this question

1 Answer 1

up vote 6 down vote accepted

This is excellent code for a Python novice.

To address your questions:

  1. Rather than x and y, try naming your variables row and col. Then it wouldn't feel unnatural to write self.state.board[row][col].
  2. In my opinion, the .neighbour() function would be better in the State class, since you're counting neighbours of a cell within that state.
  3. Your code is easy to understand, partly because the rules of the game are well known. However, you should still write docstrings for your functions. It's not obvious, for example, what to pass for the state parameter to the Game constructor unless you read the code. (Should I pass a 2D array of booleans?)
  4. You could use list comprehensions, but the current code is not bad either.
  5. I would rename both of your display(self) functions to __str__(self). In Game, that function would become

    def __str__(self):
        return str(self.state)
    

    Then you can just print my_game.

share|improve this answer
    
Thanks! Especially the __str__ method is really useful. :) –  timvermeulen Feb 5 at 16:28

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.