patternpythonMinor
Python Implementation - Conway's Game of Life
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:
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]
- 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 1000would 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.
I would also recommend trimming some of the comments to be more concise:
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
and now use that everywhere you need the string. Default
I also removed
Likewise, you don't need
Before we move on though, you should not be using a blanket
Next, your
That said, I would move your line where you read the file, back up into the
This
As for your matrix reading, it could be simplified a bit. Instead of using a loop to split and call
you can do
But now you can make the lines before and after this even simpler, by adding 0 at either end:
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
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
>>> 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 constantDEFAULT_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.