5
\$\begingroup\$

Conway's Game of Life is a common problem. I had to write a version of it to get my current C# job, so I thought it would be a nice way to get into Python (especially as the example in the book are a little too simple for my liking)

I've slightly abused the unit tests to print out the grids, normally I would not use a unit test as a functional test!

I'm not sure if I have grasped the "pythonic" principles so please let rip.

from random import randint


class Engine(object):

_grid = []

def __init__(self, rows, cols):
    self._rows = rows
    self._cols = cols

def seed(self, grid):
    """ Add a 'seed' grid; this allows adding known patterns
    :param grid: a 2 dimensional list containing only 1 or 0
    :return: null
    """
    self._grid = grid
    return

def generate(self):
    """ Using an existing 'seed' grid, follow the rules to change the
    values in the list
    :return: a 2 dimensional list containing only 1 or 0
    """

    # NB: if the system was not set up with a 'seed' pattern, generate one.
    if len(self._grid) == 0:
        self._grid = [[randint(0, 1) for row in range(self._rows)]
                     for col in range(self._cols)]

    generated = [[0 for row in range(self._rows)]
                 for col in range(self._cols)]

    for y in range(self._cols):
        for x in range(self._rows):

            neighbours = self.__neighbourCount(x, y)

            # Handles
            #  Rule 1 - Any live cell with fewer than two live neighbours
            #  dies, as if caused by under-population (by defaulting to 0)
            #  Rule 2 - Any live cell with two or three live neighbours
            #  lives on to the next generation.
            #  Rule 3 - Any live cell with more than three live neighbours
            #  dies, as if by overcrowding (by defaulting to 0)
            if ((self._grid[y][x] == 1) and
                    (neighbours == 2 or neighbours == 3)):
                generated[y][x] = 1

            # Rule 4 - Any dead cell with exactly three live neighbours
            # becomes a live cell
            if self._grid[y][x] == 0 and neighbours == 3:
                generated[y][x] = 1

    self._grid = generated
    return generated

def __neighbourCount(self, x, y):
    """ Return the sum of the value of neighbours of a cell in the grid
    :param x: the X coordinate index of the cell
    :param y: the Y coordinate index of the cell
    :return:
    """
    count = 0
    for row in [-1, 0, 1]:
        for col in (-1, 0, 1):
            if (not row == col == 0 and
                (0 <= x + row < self._rows and
                 0 <= y + col < self._cols)):
                count += self._grid[(y + col) % self._cols][(x + row) % self._rows]

    return count

Unit tests:

from nose.tools import *
from GameOfLife.engine import Engine

def test_GenerateWithOscillatorSeed():

rows = 10
cols = 10
input = [[0 for row in range(rows)] for col in range(cols)]

input[2][1] = 1
input[2][2] = 1
input[2][3] = 1

expected = [[0 for row in range(rows)] for col in range(cols)]
expected[1][2] = 1
expected[2][2] = 1
expected[3][2] = 1

engine = Engine(rows, cols)
engine.seed(input)
actual = engine.generate()

print 'INPUT ----------------------------------------------'
print(formatPrintOut(input))
print ''
print 'EXPECTED ----------------------------------------------'
print(formatPrintOut(expected))
print ''
print 'ACTUAL ----------------------------------------------'
print(formatPrintOut(actual))

assert actual == expected

# engine.seed(actual)
regenerated = engine.generate()

print ''
print 'REGENERATED ----------------------------------------------'
print(formatPrintOut(regenerated))

assert regenerated == input


def test_GenerateWithPulsarSeed():

rows = 17
cols = 17
input = [[0 for row in range(rows)] for col in range(cols)]

input = [[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
         [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
         [0, 0, 0, 0, 1, 1, 1, 0, 0, 0, 1, 1, 1, 0, 0, 0, 0],
         [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
         [0, 0, 1, 0, 0, 0, 0, 1, 0, 1, 0, 0, 0, 0, 1, 0, 0],
         [0, 0, 1, 0, 0, 0, 0, 1, 0, 1, 0, 0, 0, 0, 1, 0, 0],
         [0, 0, 1, 0, 0, 0, 0, 1, 0, 1, 0, 0, 0, 0, 1, 0, 0],
         [0, 0, 0, 0, 1, 1, 1, 0, 0, 0, 1, 1, 1, 0, 0, 0, 0],
         [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
         [0, 0, 0, 0, 1, 1, 1, 0, 0, 0, 1, 1, 1, 0, 0, 0, 0],
         [0, 0, 1, 0, 0, 0, 0, 1, 0, 1, 0, 0, 0, 0, 1, 0, 0],
         [0, 0, 1, 0, 0, 0, 0, 1, 0, 1, 0, 0, 0, 0, 1, 0, 0],
         [0, 0, 1, 0, 0, 0, 0, 1, 0, 1, 0, 0, 0, 0, 1, 0, 0],
         [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
         [0, 0, 0, 0, 1, 1, 1, 0, 0, 0, 1, 1, 1, 0, 0, 0, 0],
         [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
         [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
         ]

engine = Engine(rows, cols)
engine.seed(input)
iteration1 = engine.generate()
iteration2 = engine.generate()
iteration3 = engine.generate()


print 'INPUT ----------------------------------------------'
print(formatPrintOut(input))
print ''
print 'ITERATION 1 ----------------------------------------------'
print(formatPrintOut(iteration1))
print ''
print 'ITERATION 2 ----------------------------------------------'
print(formatPrintOut(iteration2))
print ''
print 'ITERATION 3 ----------------------------------------------'
print(formatPrintOut(iteration3))
print ''
assert input == iteration3


def formatPrintOut(grid):
return '\n'.join([''.join(['{:4}'.format('.' if item == 0 else 'X')
                           for item in row])
                  for row in grid])

Finally, output proving it is working:

    PS C:> nosetests -a
INPUT ----------------------------------------------
.   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .
.   X   X   X   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .

EXPECTED ----------------------------------------------
.   .   .   .   .   .   .   .   .   .
.   .   X   .   .   .   .   .   .   .
.   .   X   .   .   .   .   .   .   .
.   .   X   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .

ACTUAL ----------------------------------------------
.   .   .   .   .   .   .   .   .   .
.   .   X   .   .   .   .   .   .   .
.   .   X   .   .   .   .   .   .   .
.   .   X   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .

REGENERATED ----------------------------------------------
.   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .
.   X   X   X   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .

      INPUT ----------------------------------------------
.   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .
.   .   .   .   X   X   X   .   .   .   X   X   X   .   .   .   .
.   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .
.   .   X   .   .   .   .   X   .   X   .   .   .   .   X   .   .
.   .   X   .   .   .   .   X   .   X   .   .   .   .   X   .   .
.   .   X   .   .   .   .   X   .   X   .   .   .   .   X   .   .
.   .   .   .   X   X   X   .   .   .   X   X   X   .   .   .   .
.   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .
.   .   .   .   X   X   X   .   .   .   X   X   X   .   .   .   .
.   .   X   .   .   .   .   X   .   X   .   .   .   .   X   .   .
.   .   X   .   .   .   .   X   .   X   .   .   .   .   X   .   .
.   .   X   .   .   .   .   X   .   X   .   .   .   .   X   .   .
.   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .
.   .   .   .   X   X   X   .   .   .   X   X   X   .   .   .   .
.   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .

ITERATION 1 ----------------------------------------------
.   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .
.   .   .   .   .   X   .   .   .   .   .   X   .   .   .   .   .
.   .   .   .   .   X   .   .   .   .   .   X   .   .   .   .   .
.   .   .   .   .   X   X   .   .   .   X   X   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .
.   X   X   X   .   .   X   X   .   X   X   .   .   X   X   X   .
.   .   .   X   .   X   .   X   .   X   .   X   .   X   .   .   .
.   .   .   .   .   X   X   .   .   .   X   X   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .
.   .   .   .   .   X   X   .   .   .   X   X   .   .   .   .   .
.   .   .   X   .   X   .   X   .   X   .   X   .   X   .   .   .
.   X   X   X   .   .   X   X   .   X   X   .   .   X   X   X   .
.   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .
.   .   .   .   .   X   X   .   .   .   X   X   .   .   .   .   .
.   .   .   .   .   X   .   .   .   .   .   X   .   .   .   .   .
.   .   .   .   .   X   .   .   .   .   .   X   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .

ITERATION 2 ----------------------------------------------
.   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .
.   .   .   .   X   X   .   .   .   .   .   X   X   .   .   .   .
.   .   .   .   .   X   X   .   .   .   X   X   .   .   .   .   .
.   .   X   .   .   X   .   X   .   X   .   X   .   .   X   .   .
.   .   X   X   X   .   X   X   .   X   X   .   X   X   X   .   .
.   .   .   X   .   X   .   X   .   X   .   X   .   X   .   .   .
.   .   .   .   X   X   X   .   .   .   X   X   X   .   .   .   .
.   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .
.   .   .   .   X   X   X   .   .   .   X   X   X   .   .   .   .
.   .   .   X   .   X   .   X   .   X   .   X   .   X   .   .   .
.   .   X   X   X   .   X   X   .   X   X   .   X   X   X   .   .
.   .   X   .   .   X   .   X   .   X   .   X   .   .   X   .   .
.   .   .   .   .   X   X   .   .   .   X   X   .   .   .   .   .
.   .   .   .   X   X   .   .   .   .   .   X   X   .   .   .   .
.   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .

ITERATION 3 ----------------------------------------------
.   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .
.   .   .   .   X   X   X   .   .   .   X   X   X   .   .   .   .
.   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .
.   .   X   .   .   .   .   X   .   X   .   .   .   .   X   .   .
.   .   X   .   .   .   .   X   .   X   .   .   .   .   X   .   .
.   .   X   .   .   .   .   X   .   X   .   .   .   .   X   .   .
.   .   .   .   X   X   X   .   .   .   X   X   X   .   .   .   .
.   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .
.   .   .   .   X   X   X   .   .   .   X   X   X   .   .   .   .
.   .   X   .   .   .   .   X   .   X   .   .   .   .   X   .   .
.   .   X   .   .   .   .   X   .   X   .   .   .   .   X   .   .
.   .   X   .   .   .   .   X   .   X   .   .   .   .   X   .   .
.   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .
.   .   .   .   X   X   X   .   .   .   X   X   X   .   .   .   .
.   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .
.   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .   .

.
----------------------------------------------------------------------
Ran 2 tests in 0.082s

OK
Improve this question
\$\endgroup\$
1
  • \$\begingroup\$ Welcome to Codereview! Just check your indentation, as indentation is significant in Python and yours is messed up (class needs indents after it) (hint: Control-k allows for easy indenting) \$\endgroup\$
    – Caridorc
    Nov 5, 2015 at 14:50

1 Answer 1

Reset to default
4
\$\begingroup\$

Nice. There's really not a lot to complain about, but I'll focus on the "Python" stuff.

Important: You've got a bug

Have you tried instantiating two different boards at the same time? You've (probably accidently) declared _grid = [] at the class level so all instances share the same grid state (edit: I don't think that's technically true, see final note); get rid of that line and instead in the constructor add self._grid = [].

Tweak the naming

Because Python is dynamically typed, you don't have types littering your code: great. But you don't have types which clarify otherwise slightly ambiguous variable names: groan.

What do I mean by slightly ambiguous? Look at the constructor. I see the arguments are rows and cols. What you mean is number of rows, but the name suggests that perhaps it could take an actual collection of rows. So to figure out which, I had to keep reading and see how it was used; wasn't too hard, but if you name the parameter num_rows the intent becomes more clear.

For similar reasons, the interface for seed is a bit unclear. It's really a setter for _grid, so I'd just call it set_grid. This then begs the question if you're relying on your client knowing too much about the internal representation of an Engine object's grid. Probably. But for a small program, I wouldn't worry too much about it.

On being Pythonic

This line right here tells me that you've totally gotten into the Python groove. Nice.

generated = [[0 for row in range(self._rows)]
                 for col in range(self._cols)]

In a list comprehension or for loop where the name of the iterated element isn't referenced, some people prefer to deemphasize the name because it isn't used. They'll write

[0 for _ in range(self.num_rows)]

rather than

[0 for row in range(self.num_rows)]

They'd argue that row doesn't add value for the reader. I'm not telling you to do this, but it's something to think about, and something you might see others do.

Returns

I've got one uncontroversial opinion and one possibly controversial opinion.

See the return statement at the end of seed? As an uncontroversial matter of good style, get rid of it. It's just not necessary.

See the return at the end of generate ... tough call. I'm not sure it belongs there. The FP-enthusiast in me likes it, the OOP-side of me thinks it doesn't belong. Because of the way you've written the rest of the class, I hesitantly conclude that you shouldn't return any value from generate. I don't know how strongly you feel about that--removing the return value certainly has a big impact on calling code. Think about it (or not! :D).

EDIT and Final note: C# --> Python

After looking at it some more, I think the class level _grid is indeed attached but unused, and your normal code works (but only if you call seed first). Bottom line, the object system works a lot differently in Python because you can attach a member to an instance at any time. You probably should only intentionally do this in the constructor, but you might unintentionally do it and then notice strange/bugged behavior.

Here's some code that might front-load some of the surprising behavior:

class Person(object):
    location = "France"  # class variable, is this meaningful?
    def __init__(self, name, location):
        self.name = name
        self.location = location  # assign to the instance, not the class
    def __str__(self):
        return "<Person %s, in %s>" % (self.name, self.location)
    def attach_age(self, age): # don't ever write stupid stuff like this
        self.age = age  # instance probably didn't have 'age' prior to this method call

if __name__ == '__main__':
    alice = Person("Alice", "USA")
    bob = Person("Bob", "Germany")
    dave = Person("Dave", "China")

    print alice
    print bob

    print Person.location  # no error, but what does this even mean?

    alice.age = 24  # don't do stuff like this on purpose
    dave.attach_age(42)  # you're in trouble

    print alice.age
    print dave.age
    print bob.age  # error; potentially surprising
Improve this answer
\$\endgroup\$
3
  • \$\begingroup\$ Thanks very much for taking the time to review, I really appreciate it. \$\endgroup\$
    – scudsucker
    Nov 6, 2015 at 6:32
  • \$\begingroup\$ I did not realise that class level variables are static. That's a big difference, but, I should have realised that, when creating the instance of Engine did not require a new. It does still seem a little strange. "Tweak the naming" - you are correct. Coming from a strongly typed background I do tend to rely on type as a hint. "Returns" - a hangover from C#. Your first point is valid; your second is argueable. Single responsibility means "generate" should not display but maybe I should have used a getter for _grid for display purposes. \$\endgroup\$
    – scudsucker
    Nov 6, 2015 at 6:42
  • \$\begingroup\$ @scudsucker I have a love/hate relationship with Python's dynamic nature. I added an additional code example to illustrate a big difference between C# and Python. \$\endgroup\$
    – brian_o
    Nov 6, 2015 at 19:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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