Should we start to address the Horrendous Mess™, or do something else? Something else: work toward removing room knowledge from Cell.

The only thing left in Cell that is of any concern is its room instance variable. All the rest is __repr__ and coordinate stuff. So that could be worth going after. We want the cell not to know about its containers, because, well, mostly because we have come to think it’s ugly, and its mother dresses it funny.

The real information about rooms is already in DungeonLayout:

class DungeonLayout:
    deltas = [(-1, 0), (1, 0), (0, -1), (0, 1)]

    def __init__(self, max_x=10, max_y=10):
        self.cells = dict()
        for x in range(max_x):
            for y in range(max_y):
                self.cells[(x,y)] = Cell(x, y)
        self.rooms = []

In Cell, we have these references to the Cell.room variable:

class Cell:
    @property
    def room(self):
        return self._room

    @room.setter
    def room(self, room):
        self._room = room

    @property
    def is_available(self) -> bool:
        return self.room is None

    @property
    def is_in_a_room(self):
        return not self.is_available

There are 28 references to Cell.room’s getter, and 29 to the setter. Unwinding that is going to be tedious, unless we were to accept a monstrous refactoring of some kind. The nature of how I like to work, and of what I try to communicate—at most anything can be done in small steps—militates against that.

One possibility might be to change the signature of the room properties not to be properties, or perhaps to add new methods that take the layout as a parameter. Another is to put our shim in there, even though in my youthful enthusiasm I removed it this morning. With that in place we can do fifty or a hundred commits to get the job done.

Or we can take it as written, from the preceding five days, and just go ahead and see about doing it all in one massive refactoring.

Or, we could do it that way, wait a month and then say we did it in small steps.

Those options notwithstanding, a more basic question is how to move those two questions, and similar ones, over to the Layout. Layout has a list of rooms. What is a Room, anyway?

class Room:
    def __init__(self, cells, name=''):
        self.cells:list[Cell] = cells
        self.name = name
        self.color = "grey22"
        for cell in self.cells:
            cell.room = self

Nothing much to see there, just a few methods that we use for embedding rooms in other rooms. If we were to change the list to a set, we could quickly answer whether a given Cell is in the Room. The Layout could iterate the rooms to answer the question, or it could maintain a separate structure for speed.

Let’s see if we can convert that list to a set without anything exploding.

class Room:
    def __init__(self, cells, name=''):
        self.cells:set[Cell] = set(cells)
        self.name = name
        self.color = "grey22"
        for cell in self.cells:
            cell.room = self

A dozen tests fail. We’ll look but I hypothesize that the tests are expecting cells to come out in some order. Here’s one:

class Dungeon:
    def populate(self):
        dot_room = random.choice(self.rooms)
        dot_cell = random.choice(dot_room.cells)
        self.place_player_at(dot_cell)

That expects a list. Let’s change cells to return a list. First, revert. Back to green.

Unfortunately folx are referring to the member variable, which won’t do. We’ll rename cells locally, add the property.

class Room:
    def __init__(self, cells, name=''):
        self._cells:set[Cell] = set(cells)
        self.name = name
        self.color = "grey22"
        for cell in self._cells:
            cell.room = self

    def __iter__(self):
        return iter(self._cells)

    @property
    def cells(self):
        return list(self._cells)

Are people using the __iter__? Two tests are and it seems legitimate to be able to iterate and do in. Is there a way to tell in that we are set-like? Yes, implement __contains__. Let’s do.

    def __contains__(self, cell):
        return cell in self._cells

That works, and I briefly removed the __iter__ to be sure that the __contains__ is working. We can commit: Room’s cells stored as set and work properly.

I still wonder why the property can’t just return a set. Let’s see what’s failing.

There’s a call to random.choice. We could fix that in place. What else?

If users want a list, let’s leave it returning a list. It’s the polite thing to do anyway.

Now, in preparation, let’s put the available and in room methods into DungeonLayout:

I think we’d better test them. Perhaps we can find some existing tests to modify.

    def test_available(self):
        layout = DungeonLayout(5, 5)
        assert layout.at(3,3).is_available
        layout.at(3,3).room = 666
        assert not layout.at(3,3).is_available
        assert layout.at(3,3).is_in_a_room

Perfect, we can just send those checks to Layout and make them work.

    def test_available(self):
        layout = DungeonLayout(5, 5)
        assert layout.is_available((3,3))
        layout.at(3,3).room = 666
        assert not layout.is_available((3,3))
        assert layout.at(3,3).is_in_a_room

Test breaks, of course. We code:

class DungeonLayout:
    def is_available(self, xy):
        return self.at(*xy).is_available

Green. I chose to pass the tuple, because we seem to be moving in the direction that a Cell is little more than a tuple. We’ll see how this goes. Green. Commit: Layout handles is_available.

Let’s change that same test to ask for in a room:

    def test_available(self):
        layout = DungeonLayout(5, 5)
        assert layout.is_available((3,3))
        layout.at(3,3).room = 666
        assert not layout.is_available((3,3))
        assert layout.is_in_a_room((3,3))

We can make that work but it’s not a good test yet. First make it go:

class DungeonLayer:
    def is_in_a_room(self, xy):
        return not self.is_available(xy)

The test is rotten because it is assuming you can just jam in a room and a Cell is in the room, which is not the case.

Let’s add the room properly:

    def test_available(self):
        layout = DungeonLayout(5, 5)
        assert layout.is_available((3,3))
        room = Room([layout.at(3,3)])
        assert not layout.is_available((3,3))
        assert layout.is_in_a_room((3,3))

Still green. Should be committing. Commit: added is_available and is_in_a_room.

The test is still rotten: we really don’t want it to pass until it adds the room to the Layout. When we stop referring to the Cell, it will break. That’ll be a good thing.

Now can we make is_in_a_room not call down to the Cell?

We recode:

    def is_available(self, xy):
        return not self.is_in_a_room(xy)

    def is_in_a_room(self, xy):
        cell = self.at(*xy)
        return any([cell in room for room in self.rooms])

The test fails. We add the room to the layout:

    def test_available(self):
        layout = DungeonLayout(5, 5)
        assert layout.is_available((3,3))
        room = Room([layout.at(3,3)])
        layout.add_room(room)
        assert not layout.is_available((3,3))
        assert layout.is_in_a_room((3,3))

It passes. Commit: Room is_available and is_in_a_room do not rely on Cell.room.

I think that’ll be enough for an afternoon’s fiddling. We’re in a position now to modify at least some of the references to Cell.room, and can quickly add whatever else room-related may turn up into Layout.

Step by step … see you next time!