2
\$\begingroup\$

I'm looking for comments on both the micro (individual lines of code) and macro (overall design) scale. My input/output format is a single string, with newlines delimiting rows and '*'/' ' denoting alive and dead, respectively.

from itertools import product
from enum import Enum
class Direction(Enum): forwards = 1; backwards = 0

class State:
    table = {'*':True, ' ':False}

    def convert(self, grid, direction):
        if direction == Direction.forwards:
            table = self.table
        elif direction == Direction.backwards:
            table = {value:key for (key,value) in self.table.items()}
        else:
            raise ValueError("Invalid conversion direction!")

        return [[table[x] for x in row] for row in grid]

    def __init__(self, grid):
        grid = list(filter(None, grid.split('\n')))
        self.rows = len(grid); self.cols = len(grid[0])

        if not all(len(grid[i]) == self.cols for i in range(self.rows)):
            raise ValueError("Invalid game state!")

        self.grid = self.convert(grid, Direction.forwards)

    def __getitem__(self, i):
        return self.grid[i]

    def __str__(self):
        return '\n'.join(map(lambda x: ''.join(x), self.convert(self.grid, Direction.backwards)))

def game(state):    
    ''' Takes a state in the input format and yields states in the output format. '''

    state = State(state)

    def alive(x,y):
        try:
            return state[x][y]
        except IndexError:
            return False

    while True:
        state.grid = [
            [2 <= sum(alive(xx,yy) for xx, yy in product([x-1,x+1], [y-1,y+1])) <= 3
             for (y,col) in enumerate(row)]
            for (x,row) in enumerate(state.grid)
        ]
        yield str(state)
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Regarding code style, it's better to respect the PEP8 and use Flake8 to check your code.

For instance, replace:

from itertools import product
from enum import Enum
class Direction(Enum): forwards = 1; backwards = 0

by

from itertools import product
from enum import Enum


class Direction(Enum):
    forwards = 1
    backwards = 0

Regarding implementation:

When you raise an exception, always give the reason:

raise ValueError("Invalid conversion direction {0!r}! Only backwards or forwards.".format(direction))

In fact, convert() function could be “protected" because it is not used outside the class. And there is no need to check that the direction value is something else than backwards and forwards.

By convention, you could rename the alive function to is_alive because it is a predicate (it returns a Boolean). This function should be a method of State class because it use state variable.

For the same reason, the game function should also be a method. I don't like its name because we cannot understand that it's a generator. Something like next_state could be better.

At the end you calculate the sum of alive() values which are booleans. I consider this to be a bad practice: it's better to use 0 and 1.

\$\endgroup\$

Your Answer

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

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