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?

share|improve this question
1  
There seems to be a bug: the x and y variables passed to State will be ignored as you are redefining them when you use them as loop variables. –  Stuart Jul 27 at 19:02
    
@Stuart I'll admit that it's confusing that I used the variables x and y twice, but it's not a bug. The x and y variables passed to State signify where the passed positions should be placed on the board and they are used in for cell in active_cells: board[cell[1] + y][cell[0] + x] = True. Those variables are not affected by the x and y variables I used in the for loops. –  timvermeulen Jul 27 at 21:15
    
It is a bug and they are affected by the loops. Try inserting print x, y after the loop to see this. –  Stuart Jul 27 at 21:22
    
@Stuart Oh, you are right. Changing the order of the code in __init__ should fix it, though choosing different variable names would be better. Thanks! –  timvermeulen Jul 27 at 22:16

1 Answer 1

up vote 8 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.