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

Python Implementation - Conway's Game of Life

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

Problem

This is my implementation of Conway's Game of Life in Python. Now since I am a novice coder, naturally I have some key doubts:

  • The usage of idioms and code redundancies - Are there any small fragments of the program which can be better written?



  • The usage of sys.argv - Is my usage of system arguments acceptable? Eg: $ python 02 1000 would mean taking input from 'input02.txt' and running until 1000 generations.



  • Finally, in the get_input_matrix method, I use several reversals and appends in order to add empty rows and columns all around the input matrix. What could be better ways to approach this?



Along with all these, I am also wondering if variables are aptly named and the code is well-formatted/well-written, etc.

```
#!/usr/bin/python

from time import sleep
import sys

# This method is used to input the contents of the input file.
# If a matrix:
# 0 0
# 0 1
# is present in the input file,
# this generates: (adds 'buffer region' on all sides of the input)
# 0 0 0 0
# 0 0 0 0
# 0 0 1 0
# 0 0 0 0
# and returns it to the program.
def get_input_matrix(f='input.txt'):
# Attempt opening user's input file.
# If gives error, use default input file.
try:
inputFile = open(f, 'r')
except:
print r"Error: Invalid filename. Using 'input.txt'"
inputFile = open('input.txt', 'r')
finally:
matrix = inputFile.readlines()
temp = []
for line in matrix:
temp_line = [0]
for element in line[:-1].split(' '):
temp_line.append(int(element))
temp_line.append(0)
temp.append(temp_line)
buffer_array = [0 for i in range(len(temp_line))]
temp.append(buffer_array)
temp.reverse()
temp.append(buffer_array)
temp.reverse()
return temp

# Prints matrix
# 0 1
# 1 0
# as
# *
# *
def print_matrix(matrix, height, width):
for i in range(1, height-1):
for j in range(1, width-1):
if matrix[i][j]

Solution

You have extensive commenting about the usage and intent of functions, but they're just comments. You should make docstrings. Docstrings are like comments, but programmatically accessible.

>>> def help_me():
    """Isn't this helpful?"""

>>> help(help_me)
Help on function help_me in module __main__:

help_me()
    Isn't this helpful?


I would also recommend trimming some of the comments to be more concise:

"""
This method is used to input the contents of the input file.

0 0
0 1

Adds 'buffer region' on all sides of the input and generates:

0 0 0 0
0 0 0 0
0 0 1 0
0 0 0 0
"""


Next, using a default input is a great idea for when errors occur, but you've made a couple of mistakes in how you do this. First of all, you're defining f='input.txt.' as a default filename. That's a good practice, except that you've hardcoded the name in multiple places including using it as a fallback value. Instead of what you're doing now, I'd have a constant

DEFAULT_INPUT = 'input.txt'


and now use that everywhere you need the string. Default f to an empty string which will just fail and trigger the same error as an invalid filename. Here's how that change looks:

def get_input_matrix(f=''):
    """Attempt opening user's input file, fall back on default file."""

    try:
        inputFile = open(f)
    except:
        print "Error: Invalid filename. Using '{}'".format(DEFAULT_INPUT)
        inputFile = open(DEFAULT_INPUT)


I also removed 'r' from open, since r is the default value anyway there's no need to state it explicitly. Though being explicit is usually good, in this case inputFile is only used for a single line. And it's unambiguously just to be read from. If it was hard to discern what the intent was, using an explicit 'r' would clarify, but it's easy to see the intent without it here.

Likewise, you don't need r before your string. It's only necessary for preventing escape characters affecting the resulting string, but since you have no backslashes this is not an issue here.

Before we move on though, you should not be using a blanket except. This will mean that any error whatsoever can be caught and ignored. What if the user is passing actual file objects instead of strings? Surely that's a foolish enough mistake that you want to highlight it. Where possible, you should except a specific error. In this case, except IOError. This means that IOErrors (raised by invalid file names) will open your default file, but all other errors will be raised as normal.

Next, your finally block. It's not really necessary. You may have misunderstood what finally is for, but it ensures that no matter what happens in your try and except blocks, the finally block always gets executed. It's usually used for things like closing up files and other resources that shouldn't be left open. However you're using it as if everything is running smoothly. You don't need this block at all, to be honest. Just leave it as regular code.

That said, I would move your line where you read the file, back up into the try and except. And instead of using open, use what's called a context manager.

try:
        with open(f) as inputFile:
            matrix = inputFile.readlines()

    except IOError:
        print "Error: Invalid filename. Using '{}'".format(DEFAULT_INPUT)
        with open(DEFAULT_INPUT) as inputFile:
            matrix = inputFile.readlines()


This with syntax is similar to open except that it will always ensure that files are properly closed, no matter what happens. You don't actually remember to call inputFile.close(), so using with would at the very least solve that problem.

As for your matrix reading, it could be simplified a bit. Instead of using a loop to split and call int on each element, you could use map. map calls a function on each element of a collection, so instead of

for element in line[:-1].split(' '):
    temp_line.append(int(element))


you can do

temp_line += map(int, line[:-1].split(' '))


But now you can make the lines before and after this even simpler, by adding 0 at either end:

temp = []
for line in matrix:
    temp_line = [0] + map(int, line[:-1].split(' ')) + [0]
    temp.append(temp_line)


At this point, we have a prime candidate for a list comprehension. List comprehensions are like for loops collapsed into a single expression that build a list. You can basically create your full temp list in one go, just by rearranging the above lines to this:

temp = [[0] + map(int, line[:-1].split(' ')) + [0] for line in matrix]


That line essentially does the same as the above loop, but faster and neater.

Also you don't need to reverse the list to add a line to the start. Use list.insert(0, line) to add an element at index 0, ie. the start.

Code Snippets

>>> def help_me():
    """Isn't this helpful?"""

>>> help(help_me)
Help on function help_me in module __main__:

help_me()
    Isn't this helpful?
"""
This method is used to input the contents of the input file.

0 0
0 1

Adds 'buffer region' on all sides of the input and generates:

0 0 0 0
0 0 0 0
0 0 1 0
0 0 0 0
"""
DEFAULT_INPUT = 'input.txt'
def get_input_matrix(f=''):
    """Attempt opening user's input file, fall back on default file."""

    try:
        inputFile = open(f)
    except:
        print "Error: Invalid filename. Using '{}'".format(DEFAULT_INPUT)
        inputFile = open(DEFAULT_INPUT)
try:
        with open(f) as inputFile:
            matrix = inputFile.readlines()

    except IOError:
        print "Error: Invalid filename. Using '{}'".format(DEFAULT_INPUT)
        with open(DEFAULT_INPUT) as inputFile:
            matrix = inputFile.readlines()

Context

StackExchange Code Review Q#119826, answer score: 3

Revisions (0)

No revisions yet.