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

Game of Life, a shorter story

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

Problem

I've been glancing at Game of Life for quite a while, but been reluctant towards graphical packages like pygame. But today I finally got over myself and did it.

Did you know that there the "problem" computationally only depends on one condition? I didn't, and followed the conditions of the game at first.

At first I used list to contain the coordinates if alive bricks or plebes as I call them. But I quickly realized that order did not matter and I abandoned the lists. Way are Python lists so slow?

Peer reading makes me wiser, so feel free to give any pointers.

```
class GameOfLife:
def __init__(self, size=800):
"""
The implementation is based on sets and dict, because order never
matters in this game. Sets and dicts are faster. There is no need
for complex data-structures like list of lists, order don't matter.

:param self.screen: the screen used by pygame.
:param self.white: color for pygame. alive plebs
:param self.black: color for pygame. dead plebs or "empty" area

:param self.width: screen width
:param self.size: size of a pleb.
:param self.alive: alive plebs.
:param self.last_config: if self.alive, has not changed, it will not
change. So this is an end condition.
"""
self.screen = pygame.display.set_mode((size, size))
self.white = (255, 255, 255)
self.black = (0, 0, 0)

self.width = size
self.size = size//100
self.brick = [self.size, self.size]
self.alive = set()
self.last_config = set()

def show(self):
pygame.Surface.fill(self.screen, self.black)
for pos in self.alive:
x, y = pos
pos = xself.size, yself.size
pygame.draw.rect(self.screen, self.white, list(pos)+self.brick, 0)
pygame.display.flip()

def setup(self):
"""
Initialize the game, i.e. sets initial alive plebs.
"""

alive

Solution

The hard coded configurations look extremely painful. A good technique in this kind of situations is to come up with a convenient way to write the configuration by humans, and some helper functions to translate the human readable configuration to the data structures needed by the program. Something like this:

glider = (
    "###",
    "  #",
    " # ",
)


And then, parse this with a helper function that takes the lines tuple and the x and y offsets where it should start drawing on the canvas.

def parse_config(x, y, lines):
    # ...


This function should create the kind of configurations that you did through no small pain.

Another variation on the same idea:

import textwrap

def normalized_ascii(text):
    return textwrap.dedent(text).strip()

glider_raw = """
    ###
      #
     #
    """

glider_ascii = normalized_ascii(glider_raw)

def parse_config(x, y, ascii):
    pass


The difference is that instead of a list, the raw configuration is a multi-line string, which might be slightly easier to type. The excess indentation indentation is easy to remove with textwrap.dedent, and the excess blank lines with strip. The parse_config helper will need to be rewritten to interpret this format. Actually, it can split the ascii input by line characters, and then it's the same format as the first example.

Make sure to unit test the helper function (parse_config) well. And then you can comfortably start using a much more user-friendly configuration syntax.

Note that it's not worthwhile to worry about the extra computation added by the configuration parser. The overhead is typically insignificant next to the main operations of the program. Making the configuration easy to write and read, at the expense of an extra parsing step is always worth it, and saves you time. This is thanks to eliminating the frustration of misconfiguration, which is a very high risk when the format is not easy to write and read.

The implementation should be written in terms of the domain.
self.white and self.black are not terms of the domain, but implementation details. The implementation would read more naturally if you renamed these to self.empty_color and self.alive_color, respectively.

The patterns list is recreated in every call to get_connected_plebs.
This is unnecessary, as this list is a constant.
It would be better to make it a static attribute of the class.

Also, patterns is not a great name. These are coordinate offsets. So I'd call them offsets. And when iterating over it, instead of i and j as the loop variables, which are commonly used in counting loops, I'd call them dx and dy.

Instead of cache.update([n for n in neighbors]), you can make a copy of a list easier with cache.update(neighbors[:]).
But do you actually need a copy here?
Wouldn't simply cache.update(neighbors) be enough?

Code Snippets

glider = (
    "###",
    "  #",
    " # ",
)
def parse_config(x, y, lines):
    # ...
import textwrap

def normalized_ascii(text):
    return textwrap.dedent(text).strip()

glider_raw = """
    ###
      #
     #
    """

glider_ascii = normalized_ascii(glider_raw)

def parse_config(x, y, ascii):
    pass

Context

StackExchange Code Review Q#144259, answer score: 7

Revisions (0)

No revisions yet.