HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Game Of Life OOP Python

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
lifegamepythonoop

Problem

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 = GameOfLi

Solution

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"

Code Snippets

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

board = Board(15)
board.fill(row=7, col=7)
for state in islice(GameOfLifeGenerator(board), 11):
    print(state)
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))
def __str__(self):
    return "\n".join(
        [''.join(
            ['#' if cell else '.' for cell in row]
         ) for row in self._board]
    ) + "\n"
def __str__(self):
    return "\n".join(
        map(lambda row: ''.join(
            map(lambda cell: '#' if cell else '.', row)
        ), self._board)
    ) + "\n"

Context

StackExchange Code Review Q#32044, answer score: 2

Revisions (0)

No revisions yet.