Cleaning .., and an IDEA
I did a little off-article cleaning of Cell. We’ll look at that. And I have an idea which may be good, or then again, not. Weirdly, things go better than expected.
The Issue
The Cell object is a major player in our current design. Rooms and paths are collections of Cells. Cells know what Room they are in. They can produce their neighboring Cells, including some conditions for what kind of neighbor is acceptable. You can get to a cell by arbitrary x-y offset from any other.
The Cells accomplish this by knowing their own x-y coordinates in a CellSpace instance, which is not much more than a big array of NxM Cells. And each Cell knows the CellSpace, so it can access any other Cell via x and y.
There are at least two issues with this design that might be concerning.
One concern is that it is uncommon for an element of a collection to know what collection it is in. Usually a given element can and will be in many collections, so knowing who has it is troublesome.
My view here is that our cells are explicitly and intrinsically arranged in a 2D array, in the same way that vast tracts of land are inherently adjacent to one another. As such, the substrate that holds them together is a part of the meaning of Cell. So I deem this concern to be null and void, and None for that matter.
A second concern is smaller but irritating. Once our code has a Cell, it can pretty much do anything we need, since we can ask the Cell for neighbors, or to generate a map, or to find a path, and it will happen.
But to get the first Cell, say the one at (4,5), we have to do something like this:
space = CellSpace(100,100)
cell = space.at(4,5)
Other than in tests of CellSpace itself, if there are any, we never refer to that space again. And I find that irritating, a forced reference to a class that we would otherwise not have to know about.
The Idea
So I was thinking myself to sleep last night, and I thought: what if we only had to know about Cell? What if Cell’s use of CellSpace, or whatever laces Cells together, were entirely internal to Cell?
We would still need some capabilities relating to the whole collection of Cells. We’d perhaps need to be able to clear all the cells, setting them back to a big empty array of Cells. Or maybe we’d want to just allocate a new big array of Cells.
Or—and I think this is a reach—maybe there could be any number of Cells, any number of rows and columns. Maybe Cells would just exist as needed, and Cells never referenced wouldn’t exist at all.
We have no need for the last bit, as far as I can see. A sufficiently large 2D array can contain all the cells, and a cell already knows whether it is allocated to a Room or available. So we’ll belay that last idea.
Refining the Idea
OK, what if there are class methods on Cell, including at least
Cell.create_space(max_x,max_y)? Then, instead of creating and tossing a CellSpace, we could perhaps say:
Cell.create_space(100,100)
cell = Cell(4,5)
There would be a bit of magic in there. Cell(4,5) looks like an instance creation but in fact we want always to get the same Cell instance when we ask for Cell(4,5). I think we could make this work: Python lets us implement __new__, and that could do the job, returning the “singleton” Cell(4,5) rather than a new one.
Assessing the Idea
Well. Is the total value of this idea replacing creation of a CellSpace with an equivalent line of code? Then it’s probably not really worth doing.
However, I think it will be interesting, that we’ll learn a little something, and it might be fun. So we’re going to do it anyway.
Let’s review how cells get created now and see whether we can find a smooth step by step way to do this. I’m sure we could do it all in one go, but it would be nice if the current sequence of creating a space and then asking it for a cell would do the right thing, so that we can remove those at our leisure.
I think we can leave CellSpace alone, but we need to be aware that there are two active methods in it:
class CellSpace:
def __init__(self, max_x, max_y):
self.cells: list[Cell] = list()
self.cell_array: list[list[Cell]] = list()
self.width = max_x
self.height = max_y
for x in range(max_x):
col = []
self.cell_array.append(col)
for y in range(max_y):
cell = Cell(x, y, self)
self.cells.append(cell)
col.append(cell)
def at(self, x, y) -> Cell|None:
if 0 <= x < self.width and 0 <= y < self.height:
return self.cell_array[x][y]
else:
return None
def random_available_cell(self):
available = [cell for cell in self.cells if cell.is_available]
if available:
return random.choice(available)
else:
return None
We note that creating the CellSpace creates all the Cell instances right then.
Every Cell already knows the CellSpace:
class Cell:
def __init__(self, x, y, space):
self._x = x
self._y = y
assert space is not None
self._space = space
self._room = None
I’ve been thinking that we’d allow Cell(4,5) to get the proper existing Cell instance, spoofing the __new__. Let’s not do that. Instead, let’s give Cell two new class methods, at and random_available_cell.
I wrote this little test, line by line:
def test_class_methods(self):
Cell.create_space(10, 10)
cell = Cell.at(4,5)
random_cell = Cell.random_available_cell()
assert cell is not None
assert random_cell is not None
And the code:
class Cell:
deltas = [(-1, 0), (1, 0), (0, -1), (0, 1)]
space = None
@classmethod
def create_space(cls, max_x, max_y):
from cell_space import CellSpace
cls.space = CellSpace(max_x, max_y)
@classmethod
def at(cls, x, y):
return cls.space.at(x, y)
@classmethod
def random_available_cell(cls):
return cls.space.random_available_cell()
Commit: adding space-oriented class methods.
Now I can replace a lot of CellSpace references. Tests look nicer, for example:
def test_neighbors(self):
Cell.create_space(5, 5)
cell = Cell.at(2,3)
neighbors = cell.neighbors
assert len(neighbors) == 4
for n in [Cell.at(1, 3), Cell.at(3, 3), Cell.at(2, 2), Cell.at(2, 4)]:
assert n in neighbors
I am tempted to create a default space if there is none, but we’ll hold off on that.
In looking for things to change, I found this usage, which I had forgotten about:
main.py
def make_a_cave_room(dungeon: Dungeon, maker: RoomMaker, space: CellSpace):
origin = space.random_available_cell()
size = random.randint(50, 70)
room = maker.cave(number_of_cells=size, origin=origin)
dungeon.add_room(room)
These functions can do without the space parameter now. Currently none of them are called: we use them when we fiddle with main to test different things.
Main is quickly fixed up. We’ll call a break at this point.
Summary
This went more easily than I anticipated, because of the decision not to futz with the constructor but instead implement at as a class method on Cell. The result was that instead of needing to refactor things, we were able to keep all the existing code working, plus the new code. Right now, some tests are converted and quite a few are not. The conversion isn’t quite automatic but it’s very simple and I seem to be able to multi-cursor most of it within a given file.
If we had done the new-futzing, I think this change would have been quite questionable but as it stands, it was quite straightforward. There is now no need for any class using Cell to know about CellSpace, and we could replace its mechanism with another quite readily.
This turned out better than I expected. A rare and pleasant event!
I’ll commit the works. Maybe next time, move CellSpace into the cell file. Maybe not.
See you next time!