Game Of Life OOP Python

Question

This is pretty much an OOP version of this question, with the improvements:

import copy
import shelve

class GameOfLife(object):
    def __init__(self, board, generations = 10):
        self.__board = board
        for i in range(generations):
            print(self)
            self.__nextGeneration()
    
    def __str__(self):
        string = ""
        for row in self.__board:
            for cell in row:
                if cell:
                    string += "#"
                else:
                    string += "."
            string += "\n"
        return string
    
    def __isInRange(self, row, cell):
        return 0 <= row < len(self.__board) and \
               0 <= cell < len(self.__board[0])
    
    def __countSurrounding(self, row, cell):
        SURROUNDING = ((row - 1, cell - 1),
                       (row - 1, cell    ),
                       (row - 1, cell + 1),
                       (row    , cell - 1),
                       (row    , cell + 1),
                       (row + 1, cell - 1),
                       (row + 1, cell    ),
                       (row + 1, cell + 1))
        count = 0
        for surrRow, surrCell in SURROUNDING:
            if self.__isInRange(surrRow, surrCell) and \
               self.__board[surrRow][surrCell]:
                count += 1
        return count
    
    def __nextGeneration(self):
        nextBoard = copy.deepcopy(self.__board)
        for row in range(len(self.__board)):
            for cell in range(len(self.__board[0])):
                if self.__board[row][cell] and \
                   self.__countSurrounding(row, cell) not in (2, 3):
                    nextBoard[row][cell] = 0
                elif not self.__board[row][cell] and \
                     self.__countSurrounding(row, cell) == 3:
                    nextBoard[row][cell] = 1
        self.__board = nextBoard
        
def main():
    boardFile = shelve.open("boardFile.dat")
    board = boardFile["board"]
    game = GameOfLife(board)
    
if __name__ == "__main__":
    main()

Where boardFile.dat is just a shelved file containing a boolean array.

Can anyone provide an honest code review? Thanks.

Answer 1

Object-Orientedness

I find your object-oriented design deficient. Basically, it's a bunch of procedural code that has been thrown into a class. But what can you do with the GameOfLife object that you instantiate? Nothing!

I would split the functionality into a Board class and a GameOfLife class. The Board should be able to provide its own string representation, report the contents of a cell, set the contents of a cell, check if a coordinate exists, clone itself, and perhaps load from or save to a file. The GameOfLife class would be a generator; each time you call life.next() it would produce a new board.

Your main() function should look like this:

board = Board.load('boardFile.dat')
game = GameOfLifeGenerator(board)
for _ in range(11):
    print(board)
    board = game.next()

Alternatively,

from itertools import islice

board = Board(15)
board.fill(row=7, col=7)
for state in islice(GameOfLifeGenerator(board), 11):
    print(state)

Naming

You shouldn't be using double-underscore variables. If you want to suggest that an instance variable is private, use a name with a single underscore.

You use cell to mean column number, which is confusing terminology. A "cell" should refer to one of the points on the grid; a cell has row and column coordinates. (In contrast, I don't object as much to using row as shorthand for the coordinate of a row. English is just weird that way.)

In __countSurrounding(), the SURROUNDING list shouldn't be all caps. I'd consider that to be a variable rather than a constant. (You could define it as SURROUNDING_OFFSETS = ((-1, -1), (-1, 0), ...) instead, which would be a constant.) Also consider laying out the code this way for quick visualization:

surrounding = ((row - 1, col - 1), (row - 1, col), (row - 1, col + 1),
               (row    , col - 1),                 (row    , col + 1),
               (row + 1, col - 1), (row + 1, col), (row + 1, col + 1))

Miscellaneous

In __isInRange() and __nextGeneration(), you use len(__board[0]) as the width. I would prefer that you use len(__board(row)) instead.

The __str__() function could be implemented more succinctly as

def __str__(self):
    return "\n".join(
        [''.join(
            ['#' if cell else '.' for cell in row]
         ) for row in self._board]
    ) + "\n"

or

def __str__(self):
    return "\n".join(
        map(lambda row: ''.join(
            map(lambda cell: '#' if cell else '.', row)
        ), self._board)
    ) + "\n"

Answer 2

This looks good. Here are a few (untested) comments.

• Your __str__ method could take advantage of ternary operator, list comprehension and the join method :

This :

def __str__(self):
    string = ""
    for row in self.__board:
        for cell in row:
            if cell:
                string += "#"
            else:
                string += "."
        string += "\n"
    return string

becomes this,

def __str__(self):
    string = ""
    for row in self.__board:
        for cell in row:
            string += "#" if cell else "."
        string += "\n"
    return string

this :

def __str__(self):
    string = ""
    for row in self.__board:
        string += "".join(["#" if cell else "." for cell in row])
        string += "\n"
    return string

and ultimately this :

def __str__(self):
    return "\n".join(["".join(["#" if cell else "." for cell in row]) for row in self.__board])

• Don't iterate using range(len(list)), use enumerate.

For example :

for i,row in enumerate(self.__board):
    for j,cell in enumerate(row):
        if cell:
            if self.__countSurrounding(i, j) not in (2, 3):
                nextBoard[i][j] = 0
        else:
            if self.__countSurrounding(i, j) == 3:
                nextBoard[i][j] = 1

• You could define SURROUNDING as a constant expression.

You just need to a bit of the code using it.

    SURROUNDING = ((-1, -1),
                   (-1,  0),
                   (-1, +1),
                   ( 0, -1),
                   ( 0, +1),
                   (+1, -1),
                   (+1,  0),
                   (+1, +1))
    count = 0
    for x,y in SURROUNDING:
        i,j = row+x, cell+y
        if self.__isInRange(i,j) and \
           self.__board[i][j]:
            count += 1