So yesterday I looked at the commit messages and promised to review the code today without really thinking about how I’d represent code changes to a lay audience.
What follows is my best attempt
Commit one
Title: “Creates grid in correct size”
My guess:
I mean this seems clear enough. I’m guessing there’s a function to create a grid with a size?
What I actually did:
Created two classes, Grid
and Cell
. Cell
does absolutely nothing and isn’t used. The Grid
class is used. It generates a 2-D array when its instantiated.
There’s also a test to check if the grid is the length I expect it to be, but I don’t test whether the height is correct.
Commit two
Title: “Take care of (literal) corner cases”
My guess:
Oh man I’m already unsure. I know in the Game of Life we have to deal with corners slightly differently. If you’ve made the grid as a 2-D array and try to count to the left of the left hand side, for example, you’ll get an
IndexError
. So that’s my guess? But I don’t seem to have implemented any kid of solution to non-corner cases yet.
What I actually did:
God, I’ve been annoyed about this smug, knowledge-assuming commit message since yesterday.
I added an attribute on the Grid
class called .length
, which is where I store the size of the grid. I also update .__init__
to instantiate Cell
s in every space of the grid. I also come up with a new method, .neighbour_coordinates()
, which takes an x-coordinate and y-coordinate. It returns an array containing the coordinates of the neighbours of the cell whose coordinate was passed to the method.
I’ve also written some good tests that check that the method I’ve described above returns the correct number of neighbours when the cell under investigation is in a corner. I’ve also drawn out a test Grid
into a fixture, reducing the amount of repeated code.
This is way too complex for a flippant, one-line commit message. This needs rewriting. This is a bad commit message. It’s good code though.
Commit three
Title: “Refactored to pull out neighbour objects immediately”
My guess:
I have absolutely no idea what this does
What I actually did:
- renamed
.neighbour_coordinates()
to.neighbours
- changed the method so that it returns an array of
Cell
s from theGrid
rather than an array of coordinates. I guess that’s what I meant by the commit message
Commit four
Title: “Cells die and can be brought back”
My guess:
Okay, that’s much better. In the GoL, there are some rules that define whether the state of a cell in the grid can change. We like to call these
alive
anddead
because it’s the Game of Life, but I’ve probably just set these asTrue
orFalse
values in the grid. In any case at this point I guess I’ve implemented some mechanism to kill/resurrect the cells.
What I actually did:
Exactly what I guessed! I gave the Cell
class an __init__
method which sets an .alive
attribute, defaulting to False
. There’s also a .die()
method, which changes .alive
to False
, and a .resurrect()
method which changes .alive
to True
. Nice!
Commit five
Title: “Grid can determine the fate of a cell, given its neighbours and its state”
My guess:
Ah nice! So it looks like I’ve implemented a method that takes a cell (and apparently all its neighbours? What were you doing past Jonathan?) and its current state and works out if it lives or dies in the next iteration. So I must have implemented all the rules, I guess?
What I actually did:
This! I bunged all the rules into a big old if...elif
statement. It looks like I’ve made that a method on the Grid
class (.will_live(neighbours, cell_alive)
which feels a bit odd, but overall I’m pretty happy with it. You’ll notice the method doesn’t take self
, which means it’s a static method. And generally that’s a bit of a code smell, I think.
Commit six
Title: “The Grid knows your fate!”
My guess:
Deep sigh.
Don’t code past ten folks, apparently your brain turns into a blizzard of nonsense
What I actually did:
I added a new attribute to the Cell
class called .fate
which initialises with a value of None
. I’ve also added a method to Grid
called .determine_fate(self, y_coordinate, x_coordinate)
. It takes self
so it’s an instance method, which means we’ll need a real instance of the class to use it – static methods don’t need a real example of the class to work. It returns a True
or False
which indicates whether the Cell
at the coordinates passed to the method will survive to the next iteration. To enable this, I added a second new method to the Grid
class: .get_cell_status(self, y_coordinate, x_coordinate)
. This method does what it says on the tin – tells you if the Cell
at those coordinates is currently alive or dead.
I don’t think I use the attribute I added to the Cell
class though.
Commit seven
Title: “The Grid knows the fate of all!”
My guess:
Oh for goodness sake. At a guess maybe there’s now a method on the
Grid
class that works out the state in the next iteration for every cell?
What I actually did:
Mostly that, actually. I:
- renamed
.determine_fate()
to.determine_fate_for_one()
without changing the signature[mfn]what arguments we pass to the method[/mfn] - created a new method on
Grid
,.determine_fates_of_all(self)
, which returns a 2-D array consisting ofTrue
/False
values which map to alive/dead. This is presumably the grid for the next iteration - Still did not use the
.fate
attribute on theCell
class :thinking_face:
Commit eight
Title (and body this time!):
Removed the cell I don't think the cell needs anything more than to hold its state. I may get rid of the object later and work on pure booleans
My guess:
Oh okay, so apparently I had a
Cell
class. Except the title isRemoved the cell
while the body seems to suggest I’ve not got rid of it. For goodness sake
What I actually did:
So I didn’t actually remove the Cell
for starters, but I did have such a class. Top sleuthing from me there! I did get rid of my nice getters and setters, changing the code so that I just manipulated the attributes directly. I don’t think this is really better? It feels a bit uglier, frankly, and not as clear. I’m not happy with this change, Jonathan.
Commit nine
Title: “The Grid now accepts an inital state”
My guess:
As what, a tuple? An array? What on earth does that mean?!
What I actually did:
An array of arrays, actually. I updated the code so that you could pass Grid
a 2-D array of True
/False
values representing the initial state of the Grid
. That’s probably about acceptable, though more detail would be good.
Commit ten
Title: “The world ticks. The grid is remade. Dooooooom”
My guess:
And apparently after 11pm you lose any vestige of normalcy.
What I actually did:
I updated the type hint so people using my Grid
class would know what kind of thing they needed to pass it in order to set up a particular state at runtime.
I also added a new class that kind of has to be seen to be believed:
class World:
def __init__(self, grid: Grid):
self.history = []
self.now = grid
def tick(self):
"""
This method will determine the fate of all cells in the
grid, append now to history, and form a new grid using
the predetermined fates as its initial state. So the world
turns.
:return: None
"""
self.history.append(self.now)
self.now = Grid(len(self.now.diagram), self.now.determine_fates_of_all())
Ohhhkay. So there’s now a World
in which the Grid
exists. When an instance of World
calls the method .tick(self)
– you know what. You should be able to work it out from the docstring, the little bit of writing in quotes.
No? Alright. Let me try. It looks like .history
is going to be an array of Grid
s, where .now
represents the Grid
showing the current iteration of the world. It does this by instantiating a new Grid
(and therefore new Cell
s) at the end of every .tick()
, passing the fates of every Cell
in the old Grid
as input.
Alright. That is reasonably poetic. Well done me.
November is National Blog Posting Month, or NaBloPoMo. I’ll be endeavouring to write one blog post per day in the month of November 2019 – some short and sweet, others long and boring.