7
\$\begingroup\$

I've implemented Game of Life in Python for a programming problem, in two different versions: one is a simple script and the other uses a class with a dictionary to initialize the various parameters.

Here is the version with a class:

# Object oriented implementaition of Conway's Game of life
import random
import time
import os

class GOL():
    def __init__(self, rows, cols, delay, num_generations,\
            alive_cell="*", dead_cell="."):
        self.rows = rows
        self.cols = cols
        self.delay = delay
        self.generations = num_generations
        self.alive_cell = alive_cell
        self.dead_cell = dead_cell

    def read_grid(self, array):
        """
        Reads a given grid from a text file and sanitizes it to be used with the
        script.

        Keyword arguments:
        array -- the array into which the grid is loaded.

        Using python's with keyword the values of the grid are loaded into the array
        line by line. Once the values are loaded, it checks for the boundaries and sets
        them to -1
        """
        with open("grid.txt", 'r') as f:
            for line in f:
                temp = []
                for i in range(len(line) - 1):
                    if line[i] == "*":
                        temp.append(1)
                    elif line[i] == ".":
                        temp.append(0)
                array += [temp]
        print(array)

        for i in range(len(array)):
            for j in range(len(array[0])):
                if (i == 0 or j == 0 or (i == len(array) - 1) or (j == len(array[0]) - 1)):
                    array[i][j] = -1


    def init_grid(self, array):
        for i in range(self.rows):
            single_row = []
            for j in range(self.cols):
                if(i == 0 or j == 0 or (i == self.rows - 1) or ( j == self.cols - 1 )):
                    single_row.append(-1)
                else:
                    ran = random.randint(0,3)
                    if ran == 0:
                        single_row.append(1)
                    else:
                        single_row.append(0)
            array.append(single_row)

    def start_simulation(self, cur_gen):
        """
        This function runs the simulation.
        Keyword arguments:
        cur_gen -- the array representing the current generation

        This function creates a temp array of same size as the cur_gen array with
        random values. It prints the current generation,processses the next
        generation and swaps the current genration with the next one and repeats
        the process until it has finished running the simulation for num_gen
        generations
        """
        next_gen = []
        self.init_grid(next_gen)

        for gen in range(self.generations):
            self.print_gen(cur_gen, gen)
            self.process_next_gen(cur_gen, next_gen)
            time.sleep(self.delay)

            # Swapping this generation with the next
            cur_gen, next_gen = next_gen, cur_gen
        input("Simulation finished. Press any key to exit")

    def process_next_gen(self, cur_gen, next_gen):
        """
        Keyword arguments:
        cur_gen -- array representing the current generation
        next_gen -- array representing the next generation

        Iterates over current generation array and sets the values for the
        cells in the array for the next generation by processing the neighbors
        of each cell in the current generation
        """
        for i in range(1, self.rows-1):
            for j in range(1, self.cols-1):
                next_gen[i][j] = self.process_neighbors(i, j, cur_gen)

    def process_neighbors(self, x, y, cur_gen):
       """
        Returns the value for a given cell in the next generation

        Keyword arguments:
        x -- row coordinate of the current cell
        y -- column coordinate of the current cell
        cur_gen -- array representing the current generation

        The function first iterates over all the neighbors of the given cell and
        sets the neighbor_count variable to the number of alive cells.
        It then checks the 4 rules of Conway's game of life and returns the value
        of the cell( weather it is dead or alive ).
        """
       neighbor_count = 0

        # range() method in pyhton is exclusive, therefore to select the range between
        # x-1, x+1 we need to set the right interval of the range() method to x+2
        for i in range(x-1, x+2):
            for j in range(y-1, y+2):
                if not(i == x and j == y):
                    if cur_gen[i][j] != -1:
                        # The count is incremented by whatever value is contained by the
                        # neighboring cell. This can either be 0 or 1, but the total will
                        # always reflect the number of cells alive.
                        neighbor_count += cur_gen[i][j]

        # Checking the 4 rules of game of life.
        if cur_gen[x][y] == 1 and neighbor_count < 2:
            return 0
        if cur_gen[x][y] == 1 and neighbor_count > 3:
            return 0
        if cur_gen[x][y] == 0 and neighbor_count == 3:
            return 1
        else:
            return cur_gen[x][y]

    def print_gen(self, cur_gen, gen):
        """
        Function to handle printing each generation

        Keyword arguments:
        rows -- number of rows in the array
        cols -- number of columns in the array
        cur_gen -- the array representing the current generation
        gen -- the number of the current generation

        Simple double for loop for iterating over contents of the array and
        printing the representation of alive cells (*) and dead cells (.) to
        STDOUT
        """
        os.system("clear")
        print("Conway's game of life simulation. Generation : " + str(gen + 1))

        for i in range(self.rows):
            for j in range(self.cols):
                if cur_gen[i][j] == -1:
                    print("#", end = " ")
                elif cur_gen[i][j] == 1:
                    print(self.alive_cell, end = " ")
                elif cur_gen[i][j] == 0:
                    print(self.dead_cell, end = " ")
            print("\n")

if __name__ == '__main__':
    print("Select choice : ")
    print("1: Read initial grid from file 'grid.txt'")
    print("2: Generate random grind of size 11X40")

    choice = int(input("Option: "))

    # Reading the grid from file
    if choice == 1:
        # temp list for stroring the grid from file
        sim_params = {
                "rows" : 5,
                "cols" : 10,
                "delay" : 0.1,
                "num_generations" : 2,
                "dead_cell" : " "
            }
        simulation = GOL(**sim_params)
        this_gen = []
        simulation.read_grid(this_gen)
        simulation.start_simulation(this_gen)
    elif choice == 2:
        # initalizing the starting grid of size 22X62.
        sim_params = {
                "rows" : 22,
                "cols" : 62,
                "delay" : 0.1,
                "num_generations" : 100,
                "dead_cell" : " "
            }
        simulation = GOL(**sim_params)
        cur_gen = []
        simulation.init_grid(cur_gen)
        simulation.start_simulation(cur_gen)

And the version without:

import time
import random
import os

def read_grid(array):
    """
    Reads a given grid from a text file and sanitizes it to be used with the
    script.

    Keyword arguments:
    array -- the array into which the grid is loaded.

    Using python's with keyword the values of the grid are loaded into the array
    line by line. Once the values are loaded, it checks for the boundaries and sets
    them to -1
    """
    with open("grid.txt", 'r') as f:
        for line in f:
            temp = []
            for i in range(len(line) - 1):
                if line[i] == "*":
                    temp.append(1)
                elif line[i] == ".":
                    temp.append(0)
            array += [temp]
    print(array)

    for i in range(len(array)):
        for j in range(len(array[0])):
            if (i == 0 or j == 0 or (i == len(array) - 1) or (j == len(array[0]) - 1)):
                array[i][j] = -1

def init_grid(rows, cols, array):
    """
    Creates a array of the given size filling it with alive cells at random.

    Keyword arguments:
    rows -- number of rows of the array
    cols -- number of cols of the array
    array -- the array to fill with initial values.

    It iterates over all the values possible within the given range and sets the
   boundary values to -1. Then it fills the array with random alive(1) and dead (0)
    cells.
    """
    for i in range(rows):
        single_row = []
        for j in range(cols):
            if(i == 0 or j == 0 or (i == rows - 1) or ( j == cols - 1 )):
                single_row.append(-1)
            else:
                ran = random.randint(0,3)
                if ran == 0:
                    single_row.append(1)
                else:
                    single_row.append(0)
        array.append(single_row)

def process_neighbors(x, y, cur_gen):
    """
    Returns the value for a given cell in the next generation

    Keyword arguments:
    x -- row coordinate of the current cell
    y -- column coordinate of the current cell
    cur_gen -- array representing the current generation

    The function first iterates over all the neighbors of the given cell and
    sets the neighbor_count variable to the number of alive cells.
    """
    neighbor_count = 0

    # range() method in pyhton is exclusive, therefore to select the range between
    # x-1, x+1 we need to set the right interval of the range() method to x+2
    for i in range(x-1, x+2):
        for j in range(y-1, y+2):
            if not(i == x and j == y):
                if cur_gen[i][j] != -1:
                    # The count is incremented by whatever value is contained by the
                    # neighboring cell. This can either be 0 or 1, but the total will
                    # always reflect the number of cells alive.
                    neighbor_count += cur_gen[i][j]

    # Checking the 4 rules of game of life.
    if cur_gen[x][y] == 1 and neighbor_count < 2:
        return 0
    if cur_gen[x][y] == 1 and neighbor_count > 3:
        return 0
    if cur_gen[x][y] == 0 and neighbor_count == 3:
        return 1
    else:
        return cur_gen[x][y]

def process_next_gen(rows, cols, cur_gen, next_gen):
    """
    Keyword arguments:
    rows -- number of rows in the current generation array
    cols -- number of cols in the current generation array
    cur_gen -- array representing the current generation
    next_gen -- array representing the next generation

    Iterates over current generation array and sets the values for the
    cells in the array for the next generation by processing the neighbors
    of each cell in the current generation
    """
    for i in range(0, rows-1):
        for j in range(0, cols-1):
            next_gen[i][j] = process_neighbors(i, j, cur_gen)

def print_gen(rows, cols, cur_gen, gen):
    """
    Function to handle printing each generation

    Keyword arguments:
    rows -- number of rows in the array
    cols -- number of columns in the array
    cur_gen -- the array representing the current generation
    gen -- the number of the current generation

    Simple double for loop for iterating over contents of the array and
    printing the representation of alive cells (*) and dead cells (.) to
    STDOUT
    """
    os.system("clear")
    print("Conway's game of life simulation. Generation : " + str(gen + 1))

    for i in range(rows):
        for j in range(cols):
            if cur_gen[i][j] == -1:
                print("#", end = " ")
            elif cur_gen[i][j] == 1:
                print("*", end = " ")
            elif cur_gen[i][j] == 0:
                print(".", end = " ")
        print("\n")

def start_simulation(rows, cols, cur_gen, num_gen, delay):
    """
    This function runs the simulation.
    Keyword arguments:
    rows -- number of rows in the array
    cols -- the number of columns in the array
    cur_gen -- the array representing the current generation
    num_gen -- the number of generations the simulation has to run for
    delay -- time delay between the rendering of each generation

    This function creates a temp array of same size as the cur_gen array with
    random values. It prints the current generation,processses the next
    generation and swaps the current genration with the next one and repeats
    the process until it has finished running the simulation for num_gen
    generations
    """
    next_gen = []
    init_grid(rows, cols, next_gen)

    for gen in range(num_gen):
        print_gen(rows, cols, cur_gen, gen)
        process_next_gen(rows, cols, cur_gen, next_gen)
        time.sleep(delay)

        # Swapping this generation with the next
        cur_gen, next_gen = next_gen, cur_gen
    input("Simulation finished. Press any key to exit")

# Entry point for the script
if __name__ == '__main__':

    # Setting and declaring constatns
    _delay = 0.2
    _num_gen = 100
    _rows = 0
    _cols = 0

    print("Select choice : ")
    print("1: Read initial grid from file 'grid.txt'")
    print("2: Generate random grind of size 11X40")

    choice = int(input("Option: "))

    # Reading the grid from file
    if choice == 1:
        # temp list for stroring the grid from file
        this_gen = []
        read_grid(this_gen)
        _rows = len(this_gen)

        # All rows in the grid have the same number of columns
        _cols = len(this_gen[0])

        start_simulation(_rows, _cols, this_gen, _num_gen, _delay)
    elif choice == 2:
        # initalizing the starting grid of size 22X62.
        _rows = 22
        _cols = 62

        this_gen = []
        init_grid(_rows, _cols, this_gen)

        start_simulation(_rows, _cols, this_gen, _num_gen, _delay)

I feel that the documentation might be a little extensive. Should I reduce the amount of comments in them? Also, are there any other mistakes or conventions that I missed?

\$\endgroup\$
6
\$\begingroup\$

I have not had the opportunity to run the code and haven't spotted any bugs, so this answer only covers style and design issues.

Much of the below relates to the style guide, which you should read and at least consider following. For example, you should use a consistent quote style (either ' or ", rather than the mix you currently have) and naming (e.g. the class should really be GameOfLife).


# Object oriented implementaition of Conway's Game of life

This should be a docstring, not just a comment; modules can (and should!) have docstrings too. Also, you're missing it in the non-OOP version (which should also have a docstring for the class).


imports should be in alphabetical order (and at least consistent between the two versions!):

import os
import random
import time

Having docstrings is extremely helpful, but they should generally cover interface, not implementation. Telling the user what the functions do, in broad terms, and providing details of the appropriate parameters, return values and possible errors is good. Details of how the functions work, like "Using python's with keyword", is not; this adds to the maintenance overhead (you have to change the docstring if the implementation changes, rather than only when the interface changes) and gives the user information they shouldn't even need to know.

In terms of the comments; yes, you have way too many and (more importantly) are commenting the wrong things. Comments shouldn't describe what the code does, for example:

# Swapping this generation with the next
cur_gen, next_gen = next_gen, cur_gen

is redundant; the comment provides no more information than the code itself (you can assume someone reading through your code knows as much Python as you do). Instead, comments should be used where it is not immediately obvious why the code does it.


Constants should have UPPERCLASS_WITH_UNDERSCORES names, and generally be defined at the top of the script, not in the if __name__ == '__main__': block.


Rather than take and fill an empty list, read_grid should probably take the filename and return the list, i.e.:

this_gen = []
read_grid(this_gen)

should become:

this_gen = read_grid('grid.txt')  # or use a constant e.g. FILE_NAME

Similarly, init_grid should take either the dimensions or the list; providing both is unnecessary duplication of information duplication. Why specify the size up-front, rather than just using whatever's in the file?


In general, you should iterate over sequences directly, rather than with an index, e.g.

temp = []
for i in range(len(line) - 1):
    if line[i] == "*":
        temp.append(1)
    elif line[i] == ".":
        temp.append(0)

should be:

temp = []
for char in line:
    if char == "*":
        temp.append(1)
    elif char == ".":
        temp.append(0)

You could also look into list comprehensions, which will allow you to simplify many loops, and tools like zip and enumerate.


I would be inclined to factor out most of the code under if __name__ == '__main__' into a single entry point function, and add validation for the user's inputs (see e.g. Asking the user for input until they give a valid response). Then the last section of the file simply becomes:

if __name__ == '__main__':
    main()

For the class version, I would make two class methods, from_file and from_size to create a new instance. Also, some of the setup could be factored into class attributes:

class GameOfLife:
    """Represents a Game Of Life grid."""

    ALIVE = '*'
    DEAD = '.'

    def __init__(self, array):
        ...

    def run_simulation(self, delay, num_gen):
        """Actually run your simulation."""
        ...

    @classmethod
    def from_file(cls, filename):
        """Create a new instance from a source text file."""
        ...

    @classmethod
    def from_size(cls, width, height):
        """Create a new instance of a specified size."""
        ...

    ...

This lets you factor out the common code for both choice options, i.e.:

cur_gen = []
simulation.init_grid(cur_gen)
simulation.start_simulation(cur_gen)

I would also suggest having the generations as instance attributes, rather than passing them as parameters.

\$\endgroup\$
  • \$\begingroup\$ I did look into list comprehensions, but I decided against them because there were multiple if-else conditions inside the loops which would probably make the list comprehension very hard to read. At least that was my take on it. Thanks for the remaining pointers, I'll refactor my code to reflect these changes. Once I've done that do you think it would be a good idea to post another question asking for review ? \$\endgroup\$ – Bhargav Feb 26 '15 at 14:53
  • \$\begingroup\$ The code definitely runs when provided with the right parameters, if there is some improper input given by the user to select a choice it does generate a runtime exception and exits out. That's something I need to improve upon. \$\endgroup\$ – Bhargav Feb 26 '15 at 14:54
  • \$\begingroup\$ @Bhargav sure! See e.g. meta.codereview.stackexchange.com/a/1066/32391 for guidelines on posting a good follow-up. \$\endgroup\$ – jonrsharpe Feb 26 '15 at 14:58
  • \$\begingroup\$ I'll have a followup on here soon then. Thanks for the help! \$\endgroup\$ – Bhargav Feb 26 '15 at 15:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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