in Coding, NaBloPoMo2019

NaBloPoMo #17

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 Cells 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 Cells from the Grid 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 and dead because it’s the Game of Life, but I’ve probably just set these as True or False 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 signature1what arguments we pass to the method
  • created a new method on Grid, .determine_fates_of_all(self), which returns a 2-D array consisting of True/False values which map to alive/dead. This is presumably the grid for the next iteration
  • Still did not use the .fate attribute on the Cell 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 is Removed 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 Grids, where .now represents the Grid showing the current iteration of the world. It does this by instantiating a new Grid (and therefore new Cells) 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.

Write a Comment

Comment