Let’s see if we can do this with good testing and really small steps. It could happen.

Added Post-Completion
This article produced a different story than I expected. Getting ready for improved pathfinding led me to do a very nice little refactoring. Follow along and see how it went. The rest of this article is as written, contemporaneously as usual.

My basic plan for the dungeon map is that there are these rooms, as we’ve seen, and then there will be paths between them, a single cell wide. A sub-goal may be to make the paths wander “sufficiently”, by which I mean that I don’t want them to just be straight horizontal and vertical lines.

As things stand now, the PathFinder can be given two locations in the space, and using a flood fill with backtracking, it can find a shortest path between the source and target locations. It does not stop flooding when it finds the target, which suggests an optimization we might want to provide.

Currently, rooms are created at any available cell in the space:

main,py
def main():
    bank = CellBank(64, 56)
    # random.seed(234)
    dungeon = Dungeon()
    number_of_rooms = random.randint(12, 12)
    for _ in range(number_of_rooms):
        room = Room()
        size = random.randint(90, 90)
        # creation below
        x, y = random_available_cell(bank)
        room.build(bank, size, Cell(x,y))
        dungeon.add_room(room)
    # here is where we'll create inter-room paths
    dungeon.find_path(Cell(0,0), Cell(63,55), bank)
    view = DungeonView(dungeon)
    view.main_loop()

It is likely that the code above should become a class method or method of a DungeonBuilder class, but that notion is probably for later.

However
Having said that, I realize that we’ll want to be testing our path building and that may lead us to put the code above somewhere other than in main. We’ll keep that in mind.

Our paths need to start inside the room, or at least on its periphery, and end inside the target room. I have a starting idea, which is to designate the initial Cell sent to build as the room’s “origin” and use the room origins as the source and target in the path.

And
One thinks about all kinds of design aspects as one plans. I want to note that build takes a Cell instance. It might be that we should provide x and y and build the cell inside.
Furthermore
Thinking about that makes me notice that we do not have a Complete Construction Method for Room. Instead, we create it in an empty state and then fill it in with build. Ideally, when one creates an object, it should be ready to go as soon as we get it. Sending it messages to get it in shape is error-prone, and too often results in messy code that just gets worse.

Let’s get started. We’ll try to work so that we don’t have to keep all those ideas in mind at the same time. We’ll do room origin first. It’s easy and should serve as a good warmup, plus we can use it shortly.

We’ll extend this test:

    def test_build(self):
        random.seed(234)
        bank = CellBank(64, 56)
        room = Room()
        no_return = room.build(bank, 100, Cell(32, 28))
        assert len(set(room.cells)) == 100
        assert no_return is None
        self.verify_contiguous(room)

Thus:

    def test_build(self):
        random.seed(234)
        bank = CellBank(64, 56)
        room = Room()
        no_return = room.build(bank, 100, Cell(32, 28))
        assert len(set(room.cells)) == 100
        assert no_return is None
        assert room.origin == Cell(32, 28)
        self.verify_contiguous(room)

We are not surprised to see this fail. We code:

class Room:
    def __init__(self):
        self.origin = None
        self.cells:list[Cell] = []
        r = random.randint(0, 255)
        g = random.randint(0, 255)
        b = random.randint(0, 255)
        self.color = pygame.Color(r, g, b)
        print(self.color)

    def build(self, bank, length, start_cell: Cell):
        self.origin = start_cell
        new_cell = start_cell
        for _ in range(length):
            bank.take(new_cell)
            self.cells.append(new_cell)
            new_cell = self.find_adjacent_cell(bank)
            if new_cell is None:
                return

This should pass, but I don’t like it. Green, commit: add origin to room via build.

What I don’t like is that we have to init the origin to None (or accept a warning) in the init and then set it to the true value in the build method.

I think we should bite the bullet and do the complete creation method now. How many creations of Room must we deal with? Seven references, according to PyCharm. Only four of them are creation. We’ve seen the one in main just above, and the one in the test we just changed.

Make the Change Easy
When putting in a change, Kent Beck recommends “make the change easy, ,then make the easy change”. We want to do a Change Signature on the room’s init. Let’s rearrange the code to make that easy.

From this:

    def test_build(self):
        random.seed(234)
        bank = CellBank(64, 56)
        room = Room()
        no_return = room.build(bank, 100, Cell(32, 28))
        assert len(set(room.cells)) == 100
        assert no_return is None
        assert room.origin == Cell(32, 28)
        self.verify_contiguous(room)

To this, with two Extract Variable:

    def test_build(self):
        random.seed(234)
        bank = CellBank(64, 56)
        room = Room()
        size = 100
        origin = Cell(32, 28)
        no_return = room.build(bank, size, origin)
        assert len(set(room.cells)) == 100
        assert no_return is None
        assert room.origin == Cell(32, 28)
        self.verify_contiguous(room)

Move the Room() down.

    def test_build(self):
        random.seed(234)
        bank = CellBank(64, 56)
        size = 100
        origin = Cell(32, 28)
        room = Room()
        no_return = room.build(bank, size, origin)
        assert len(set(room.cells)) == 100
        assert no_return is None
        assert room.origin == Cell(32, 28)
        self.verify_contiguous(room)

Why? Because now, when I do the Change Signature, I can provide names to be used, bank size and origin, and PyCharm will fill them in. There’s a bit more to the idea but first, I’ll do the same to the other tests. First I’ll make sure I’m still green.

I made all the room calls to build look the same, with the same temp names, bank, origin, and size, and we’re green. Commit: preparing for signature change.

Now I have one more idea that I want to mention: when we do our change signature, all the info needed for build will be passed in on init, and I’d like to build then. But the users are all also calling build. So I plan to make the real build private, and to change build to be empty. So after the change signature everything should “just work”, if all goes well.

Let’s find out.

change signature dialog

class Room:
    def __init__(self, bank, size, origin):
        self.origin = None
        self.cells:list[Cell] = []
        r = random.randint(0, 255)
        g = random.randint(0, 255)
        b = random.randint(0, 255)
        self.color = pygame.Color(r, g, b)
        print(self.color)
        self._build(bank, size, origin)

    def _build(self, bank, length, start_cell: Cell):
        self.origin = start_cell
        new_cell = start_cell
        for _ in range(length):
            bank.take(new_cell)
            self.cells.append(new_cell)
            new_cell = self.find_adjacent_cell(bank)
            if new_cell is None:
                return
            
    def build(self, bank: CellBank, size: int, origin: Cell):
        pass

I expect this to work, but with trepidation. It does. Commit: using complete construction method.

Now I can find callers of build and remove them and the method.

I do that. But then something odd happens. When I look for senders one final time after removing all of them. I get a call to build in fallback.py, which is not one of my files.

Additionally, as it was running the refactoring of init, I thought it flashed something about 382 files. Is PyCharm looking outside my project??

Scary. Anyway, test. Green. Commit.

Reflection

I think that went rather well. I hardly had to type anything. Tests never failed. All the values for the new constructor got plugged in automatically, because we put the values in temps ahead of the place where the new signature was going to go.

Very clean. Let’s sum up and then I’ll do another article.

Summary

Sometimes things go very well, and this was one of those times. It wasn’t an accident, though I’m not going to stand here going on about how clever I am. It came about because I recognized the need for a complete construction method, and since we needed an origin member, the lack of the full constructor made for a slightly messy assignment of None in the init, On another day I’d have just accepted that. Somehow, today, making the code better seemed to make sense.

Making it better required changing the signature of Room(…), via its __init__ method. PyCharm’s Change Signature refactoring allows provision of default names to plug into the parameters. So if those names are defined ahead of the call to the method, they’ll be used.

So we extracted the constants into consistently-named temps, and ensured that they were all defined ahead of the call to Room(). Then Change Signature did the job perfectly.

One other rather smooth move was to provide an empty build method, since that was still going to be following all the Room creations, and to use a private _build, called from __init__ to do the actual build. That meant that we could remove the calls to build at our leisure, although here, since there we only a few, we just did it.

On a larger project, we could have arranged all the calls over time, done the Change Signature and update to __init__, build, and _build (all in one file), and then removed the empty build calls at our convenience.

I would not argue that there is always a smooth way to do something like this. I would report that quite often, when I look for one, I find it.

This was nice. Now maybe it’s time to do the actual path work? We’ll find out in the next article. See you then!