# Game Of Life OOP Python

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.

• To create a board… import shelve; b = [[False] * 15] * 15 ; b[7][7] = True ; shelve.open('boardfile.dat')['board'] = b – 200_success Sep 30 '13 at 21:57
• Sections 2 to 5 of my answer to your previous question apply to this question too. – Gareth Rees Sep 30 '13 at 22:03

# 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"

• Shouldn't it be '#' if col else '.' ? – Josay Oct 1 '13 at 12:20

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