import random
import rlereader
class Board:
"""Handles the status of all cells."""
def __init__(self, size):
self.size = size
self.grid = self.make_blank_grid()
self.furthest_col = 0
self.furthest_row = 0
def run_turns(self, num_turns):
"""Run a the simulator for a number of turns."""
while num_turns > 0:
self.run_turn()
num_turns -= 1
def run_turn(self):
"""Run a single turn of the simulator."""
new_grid = self.make_blank_grid()
for row in range(0, self.size):
for col in range(0, self.size):
new_grid[row][col] = self.get_cell_life(row, col)
self.grid = new_grid
def toggle_cell(self, row, col):
"""Toggle the dead or alive status of a single cell."""
self.grid[row][col] = not self.grid[row][col]
def check_furthest(self, row, col):
"""Check the furthest processed cell against this one and update if we
near the edge."""
if row + 1 >= self.furthest_row:
self.furthest_row = row + 2
if col + 1 >= self.furthest_col:
self.furthest_col = col + 2
def get_cell_life(self, row, col):
"""Return whether a given cell should become dead or alive.
This may update the processed cell boundaries if neccessary."""
living_neighbours = self.count_living_neighbours(row, col)
if self.grid[row][col]:
if living_neighbours in [2, 3]:
return True
else:
self.check_furthest(row, col)
return False
else:
if living_neighbours == 3:
self.check_furthest(row, col)
return True
return False
def check_cell(self, row, col):
"""Return whether the cell is dead or alive for the current
generation."""
if row < 0:
row = self.size - 1
if row > self.size - 1:
row = 0
if col < 0:
col = self.size - 1
if col > self.size - 1:
col = 0
return self.grid[row][col]
def count_living_neighbours(self, row, col):
"""Find how many neighnours of a given cell are alive."""
active_count = 0
to_check = [
(row - 1, col - 1), # Top left
(row - 1, col), # Top
(row - 1, col + 1), # Top right
(row, col - 1), # Left
(row, col + 1), # Right
(row + 1, col - 1), # Bottom left
(row + 1, col), # Bottom
(row + 1, col + 1) # Bottom Right
]
for crow, ccol in to_check:
if self.check_cell(crow, ccol):
active_count += 1
return active_count
def make_blank_grid(self):
"""Returns a blank grid for future use."""
grid = []
for row in range(0, self.size):
grid.append([])
for col in range(0, self.size):
grid[row].append([])
grid[row][col] = False
return grid
def load_rle_into_grid(self, rle):
"""Loads a RLE representation of a playing field into the grid.
rle should be a file like object."""
reader = rlereader.GRLEReader()
data = reader.read_rle(rle) # Returns an uncompressed series of tokens from an rle file. The rle file is opened elsewhere than this package.
self.blank_grid()
current_token = 0
current_row = 0
current_col = 0
while True:
try:
token = data[current_token]
except IndexError:
break # Out of tokens
if type(token) == rlereader.EOFToken:
break
if token.value in ['b', 'o']: # 'o' = alive' 'b' = dead
self.grid[current_row][current_col] = (token.value == 'o')
current_col += 1
if current_col > self.size - 1:
print('Too wide an import, cancelling import.')
break
if current_col >= self.furthest_col:
self.furthest_col = current_col + 2
if token.value == '$': # $ indicates end of line.
current_row += 1
if current_row > self.size - 1:
print('Too high an import, cancelling import.')
break
current_col = 0
if current_row > self.furthest_row:
self.furthest_row = current_row + 2
current_token += 1
def randomise_grid(self):
"""Change every cell in a grid to random dead or alive state."""
for row in range(0, self.size):
for col in range(0, self.size):
self.grid[row][col] = random.choice([True, False])
self.furthest_row = self.size - 1
self.furthest_col = self.size - 1
def blank_grid(self):
"""Replace the current grid with a blank grid."""
self.grid = self.make_blank_grid()
This is the business end of a Python Game Of Life simulator I wrote. Can I get some opinions of it? (I hope it's not too much, I cut out all the GUI code and rlereader because they're not too relevant. blank_grid, randomise_grid and toggle_cell are all only called from the UI code.
1 Answer 1
import random
import rlereader
class Board:
"""Handles the status of all cells."""
def __init__(self, size):
self.size = size
self.grid = self.make_blank_grid()
self.furthest_col = 0
self.furthest_row = 0
def run_turns(self, num_turns):
"""Run a the simulator for a number of turns."""
while num_turns > 0:
Use a for turn in range(num_terms) loop instead of a while loop
self.run_turn()
num_turns -= 1
def run_turn(self):
"""Run a single turn of the simulator."""
new_grid = self.make_blank_grid()
for row in range(0, self.size):
Just use range(self.size), the 0 is not needed.
for col in range(0, self.size):
new_grid[row][col] = self.get_cell_life(row, col)
self.grid = new_grid
def toggle_cell(self, row, col):
"""Toggle the dead or alive status of a single cell."""
self.grid[row][col] = not self.grid[row][col]
def check_furthest(self, row, col):
"""Check the furthest processed cell against this one and update if we
near the edge."""
if row + 1 >= self.furthest_row:
self.furthest_row = row + 2
if col + 1 >= self.furthest_col:
self.furthest_col = col + 2
Write it like this instead:
self.furthest_row = max(self.furthest_row, row + 2)
self.furthest_col = max(self.furthest_col, col + 2)
Also, the function updates rather then checks the furthest, a better name would be update_furthest
I don't know why you are adding 2 here, a comment explaining that would be helpful
def get_cell_life(self, row, col):
"""Return whether a given cell should become dead or alive.
This may update the processed cell boundaries if neccessary."""
living_neighbours = self.count_living_neighbours(row, col)
if self.grid[row][col]:
if living_neighbours in [2, 3]:
return True
else:
self.check_furthest(row, col)
return False
else:
if living_neighbours == 3:
self.check_furthest(row, col)
return True
return False
Write it like this instead:
living_neighbours = self.count_living_neighbours(row, col)
if self.grid[row][cell]:
new_value = living_neighbors in [2,3]
else:
new_value = living_neighbors == 3
if new_value != self.grid[row][cell]:
self.check_furthest(row, cell)
return new_value
This way we separate the furthest updating from the decision of the new value of the cell. It might also be a good idea to move the furthest updating to run_turn
def check_cell(self, row, col):
"""Return whether the cell is dead or alive for the current
generation."""
if row < 0:
row = self.size - 1
if row > self.size - 1:
row = 0
if col < 0:
col = self.size - 1
if col > self.size - 1:
col = 0
return self.grid[row][col]
Use modulous instead of all those ifs
def check_cell(self, row, col):
return self.grid[row % self.size][col % self.size]
The % divides with remainder which gives exactly the wrap around feature you want.
def count_living_neighbours(self, row, col):
"""Find how many neighnours of a given cell are alive."""
active_count = 0
to_check = [
(row - 1, col - 1), # Top left
(row - 1, col), # Top
(row - 1, col + 1), # Top right
(row, col - 1), # Left
(row, col + 1), # Right
(row + 1, col - 1), # Bottom left
(row + 1, col), # Bottom
(row + 1, col + 1) # Bottom Right
]
for crow, ccol in to_check:
if self.check_cell(crow, ccol):
active_count += 1
return active_count
You can simplify the function by replacing it with:
return sum(self.check_cell(row, col) for row, col in to_check)
I recommend moving to_check to a global constant.
def make_blank_grid(self):
"""Returns a blank grid for future use."""
grid = []
for row in range(0, self.size):
grid.append([])
for col in range(0, self.size):
grid[row].append([])
grid[row][col] = False
return grid
Write it like this instead:
def make_blank_grid(self):
"""Returns a blank grid for future use."""
grid = []
for row in range(0, self.size):
grid.append([False] * self.size)
return grid
The multiply produces a new list which is the original list repeated. But only use it on immutable values like numbers and bools. Don't use on anything that can be modified like other lists.
def load_rle_into_grid(self, rle):
"""Loads a RLE representation of a playing field into the grid.
rle should be a file like object."""
reader = rlereader.GRLEReader()
data = reader.read_rle(rle) # Returns an uncompressed series of tokens from an rle file. The rle file is opened elsewhere than this package.
self.blank_grid()
current_token = 0
current_row = 0
current_col = 0
while True:
try:
token = data[current_token]
except IndexError:
break # Out of tokens
Use for token in data instead of the while loop. For loops are almost always better then while loops.
if type(token) == rlereader.EOFToken:
break
Breaking out because of either an EOFToken or running out of tokens is strange. You should probably only have to break in case or the other not both. Also, checking the types of variables is discouraged. You shouldn't need to do that. Is rlereader your code or something else?
if token.value in ['b', 'o']: # 'o' = alive' 'b' = dead
self.grid[current_row][current_col] = (token.value == 'o')
current_col += 1
if current_col > self.size - 1:
print('Too wide an import, cancelling import.')
break
When something goes wrong, its best to throw an exception, not print a message and continue on your merry way.
if current_col >= self.furthest_col:
self.furthest_col = current_col + 2
Use you check_furthest method, so as not to duplicate logic.
if token.value == '$': # $ indicates end of line.
current_row += 1
if current_row > self.size - 1:
print('Too high an import, cancelling import.')
break
current_col = 0
if current_row > self.furthest_row:
self.furthest_row = current_row + 2
current_token += 1
def randomise_grid(self):
"""Change every cell in a grid to random dead or alive state."""
for row in range(0, self.size):
for col in range(0, self.size):
self.grid[row][col] = random.choice([True, False])
self.furthest_row = self.size - 1
self.furthest_col = self.size - 1
def blank_grid(self):
"""Replace the current grid with a blank grid."""
self.grid = self.make_blank_grid()
One additional thing to consider would be using numpy. Numpy is a python library that provides a multi-dimensional array type. Its more natural to work with then lists of lists. It also provides vector operations, which allow you to perform the same operation against all the element in the array very efficiently and with less code.