5
\$\begingroup\$

I asked for a review of my code, and after making the suggested changes I feel that now the OOP version is clearer in its meaning and overall the code looks cleaner.

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

class GameOfLife():
    FILE_NAME = "grid.txt"
    ROWS = 22
    COLS = 62
    DELAY = 0.2
    GENERATIONS = 200

    def __init__(self, rows=ROWS, cols=COLS, delay=DELAY,from_file=False, \
                num_generations=GENERATIONS, alive_cell="*", dead_cell="."):
        """
        Initializes a GameOfLife Object with the provided values
        """
        self.generations = num_generations
        self.alive_cell = alive_cell
        self.dead_cell = dead_cell
        if from_file:
            self.cur_gen = self.read_grid()
        else:
            self.cur_gen = self.init_grid()
       self.rows = len(self.cur_gen)
        self.cols = len(self.cur_gen[0])

    def read_grid(self):
        """
        Reads a given grid from a text file and sanitizes it to be used with the
        script.
        """
        array = []
        with open("grid.txt", "r") as f:
            for line in f:
                temp = []
                for char in line:
                    if char == "*":
                        temp.append(1)
                    elif char == ".":
                        temp.append(0)
                array += [temp]

        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
        return array


    def init_grid(self,rows=ROWS, cols=COLS):
        """
        Returns an array filled with random alive and dead cells of the given
        dimensions
        """
        array = []
        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)
        return array

    def process_next_gen(self):
        """
        Iterates over each element of the current generation and processes their
        neighbors.
        """
        for i in range(1, self.rows-1):
            for j in range(1, self.cols-1):
                self.next_gen[i][j] = self.process_neighbors(i, j)

    def process_neighbors(self, x, y):
        """
        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
        """
        neighbor_count = 0

        # range() method in pyhton is exclusive
        for i in range(x-1, x+2):
            for j in range(y-1, y+2):
                if not(i == x and j == y):
                    if self.cur_gen[i][j] != -1:
                        # The count is incremented by whatever value is contained by the
                        # neighboring cell.
                        neighbor_count += self.cur_gen[i][j]

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

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

        Keyword arguments:
        gen -- the number of the current generation
        """
        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 self.cur_gen[i][j] == -1:
                    print("#", end = " ")
                elif self.cur_gen[i][j] == 1:
                    print(self.alive_cell, end = " ")
                elif self.cur_gen[i][j] == 0:
                    print(self.dead_cell, end = " ")
            print("\n")

    def start_simulation(self):
        """
        This function runs the simulation.
        """
        self.next_gen = self.init_grid(self.rows, self.cols)
        for gen in range(self.generations):
            self.print_gen(gen)
            self.process_next_gen()
            time.sleep(self.DELAY)

            self.cur_gen, self.next_gen = self.next_gen, self.cur_gen
        input("Simulation finished. Press any key to exit")

def user_input(prompt):
    """
    Only accepts an input of values either 1 or 2
    """
    while True:
        try:
            choice = int(input(prompt))
        except ValueError:
            print("Please enter either 1 or 2")
            continue
        if choice < 0 :
            print("Please enter a non negative number")
            continue
        else:
            break
    return choice


def main():
    """
    Single entry point for our module
    """
    print("Select choice : ")
    print("1: Read initial grid from file 'grid.txt'")
    print("2: Generate random grind of size 11X40")

    choice = user_input("Option: ")

    # Reading the grid from file
    if choice == 1:
        simulation = GameOfLife(from_file=True)
        simulation.generations = 2
        simulation.start_simulation()
    elif choice == 2:
        simulation = GameOfLife()
        simulation.dead_cell = " "

        # Unicode for Black Square : http://www.fileformat.info/info/unicode/char/25a0/index.html
        simulation.alive_cell = u"\u25A0"
        simulation.start_simulation()
if __name__ == '__main__':
    main()

I have a few questions regarding OOP in python though. When are we supposed to use classmethods? For example, in this script, when would a classmethod be appropriate?

\$\endgroup\$
  • \$\begingroup\$ I already made some suggestions about the use of classmethods in my previous answer (which would replace simulation = GameOfLife(from_file=True) with simulation = GameOfLife.from_file(filename) and simulation = GameOfLife() with simulation = GameOfLife.from_size(rows, cols)). If you're unclear on that, you can comment on my answer and I'm happy to expand on that. See also stackoverflow.com/a/682545/3001761 \$\endgroup\$ – jonrsharpe Feb 27 '15 at 9:38
  • \$\begingroup\$ pedant alert you've defined a "constant" FILE_NAME, but then used the string literal "grid.text" in init_grid . \$\endgroup\$ – psaxton Jul 7 '15 at 4:34
3
\$\begingroup\$

First off, the line class GameOfLife(): should be changed to class GameOfLife:.

Secondly, using os.system("clear") isn't very portable. A more portable method would be os.system("cls" if os.name == "nt" else "clear"). If you want re-usability, you can also put this into a function, like this:

def clear():
    os.system("cls" if os.name == "nt" else "clear")

You also don't need to surround conditions in if statements with parentheses. For example if(i == 0 or j == 0 or (i == rows - 1) or ( j == cols - 1 )): should be changed to i (i == 0 or j == 0 or (i == rows - 1) or ( j == cols - 1 ):.

Finally, some of your variable names aren't so great. For example, i, j, or array are not so great. Variable names like these should be more descriptive and describe what the purpose of the variables are.

\$\endgroup\$
3
\$\begingroup\$

First of all I would like to show what a typical GoL code separation in Python looks like :

Setup

import random
from collections import defaultdict

printdead, printlive = '-#'
maxgenerations = 3
cellcount = 4,4
celltable = defaultdict(int, {
 (1, 2): 1,
 (1, 3): 1,
 (0, 3): 1,
 } ) # Only need to populate with the keys leading to life

Start States

u = universe = defaultdict(int)
u[(1, 0)], u[(1, 1)], u[(1, 2)] = 1, 1, 1

Main loop

for i in range(maxgenerations):

User interface

print "\nGeneration %3i:" % ( i, )
for row in range(cellcount[1]):
    print "  ", ''.join(str(universe[(row,col)])
                        for col in range(cellcount[0])).replace(
                            '0', printdead).replace('1', printlive)

Creating a new Generation

nextgeneration = defaultdict(int)
for row in range(cellcount[1]):
    for col in range(cellcount[0]):
        nextgeneration[(row, col)] = celltable[
            ( universe[(row, col)],
              -universe[(row, col)] + sum(universe[(r, c)]
                                         for r in range(row-1, row+2)
                                         for c in range(col-1, col+2) )
            ) ]
universe = nextgeneration

Results

You can run this demo online at : http://www.skulpt.org/ with results :

Generation   0:
   ----
   ###-
   ----
   ----

Generation   1:
   -#--
   -#--
   -#--
   ----

Generation   2:
   ----
   ###-
   ----
   ----

Second of all, to

When are we supposed to use classmethods?

Come to a compromise between writing GOD object or no methods at all. A good way is to implement code skeleton that does the minimum and then look what else is required.

In typical OOP language your class is a data structure first. Methods give it's data desired behavior ex: describe how to compare items or serialize them.

For example, in this script, when would a classmethod be appropriate?

It is important that you reuse existing patterns. For example it would be natural to assume that if you have a GoL universe statespace class and you wanted to print it out, then you could just by doing print gol. This is archived with overwriting class method str.

def __str__(self):
     return "foo"
\$\endgroup\$
3
\$\begingroup\$

Overall I think your code looks good, but I have some more small suggestions:

  • Define constants for "magic" values that appear throughout your code, e.g. ALIVE, DEAD and INVALID for 1, 0 and -1 (in the appropriate contexts, of course).
  • Be consistent in how you use spaces. For example, you use both x+1 and x + 1, and in one place you put spaces on the inside of parentheses.
  • Simplify the nested loops. Some of them are inefficient and some could be written so that the communicate their purpose more clearly:

For example this:

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

could be turned into this:

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

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

And this:

for i in range(x-1, x+2):
    for j in range(y-1, y+2):
        if not(i == x and j == y):
            if self.cur_gen[i][j] != -1:
                # The count is incremented by whatever value is contained by the
                # neighboring cell.
                neighbor_count += self.cur_gen[i][j]

into this:

neighbor_count = sum(self.cur_gen[i][j] 
                        for i in [x - 1, x + 1] 
                        for j in [y - 1, y + 1] 
                        if self.cur_gen[i][j] != -1)
\$\endgroup\$

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.