Hello, loves!

We’ll glance at Flooder, to see if it needs any improvement (it surely well), then improve it. (Brief oblique cut at LLMs, oligarchs, etc.)

I am not ashamed to say that I’m pleased with how the change to Flooder went in. We had a fairly sophisticated condition for selection: select cells such that Dot could pass between them, which sounds simple until you realize that that condition comes down to the cells are in the same room, or they are in different rooms and there is a passage at that location. As things stood, Flooder’s select didn’t even have access to the parent cell, only the new possibility, but it all came down to just a few lines.

When that happens, in the old days, we thought of it as the objects “helping us”. We were big on metaphor back in those days, and there was little harm in anthropomorphizing. Today, with the text-extruding machines being so glib, it’s rather dangerous to imagine that there is a person in there helping you.

Spoiler: there isn’t a person in there. There is an oligarch consuming your money, your water, your air, your very soul. But I digress.

So I’m pleased with Flooder’s capability. Let’s see how pleased we should be with its code. Here it is:

class Flooder:
    def __init__(self, *, layout, origin):
        self._layout = layout
        self._delivered = None
        self._to_be_delivered = None
        self._origin = origin
        self._select = lambda cell, parent: True
        self._next_value = lambda cell, value: value + 1
        self._initial_value = 0
        self._randomness = 0.0

    # fluent interface

    def available(self):
        self.select(lambda cell, parent: self._layout.is_available(cell))
        return self

    def in_any_room(self):
        self.select(lambda cell, parent: self._layout.is_in_a_room(cell))
        return self

    def select(self, function):
        self._select = function
        return self

    def next_value(self, function):
        self._next_value = function
        return self

    def initial_value(self, result):
        self._initial_value = result
        return self

    def randomness(self, fraction):
        self._randomness = fraction
        return self

    # flooding

    def flood(self):
        self._to_be_delivered = {self._origin: self._initial_value}
        self._delivered = set()
        while self._to_be_delivered:
            current_cell, current_value = self._next_to_deliver()
            yield current_cell, current_value
            self._enqueue_relevant_neighbors(current_cell, current_value)

    def _next_to_deliver(self):
        current_cell = next(iter(self._to_be_delivered))
        current_value = self._to_be_delivered.pop(current_cell)
        self._delivered.add(current_cell)
        return current_cell, current_value

    def _enqueue_relevant_neighbors(self, current_cell, current_value):
        for neighbor in self._relevant_neighbors(current_cell):
            self._to_be_delivered[neighbor] = self._next_value(current_cell, current_value)

    def _relevant_neighbors(self, cell):
        return (neighbor
                for neighbor in self._get_neighbors(cell)
                if self._is_relevant(neighbor, cell))

    def _is_relevant(self, neighbor, parent):
        return (neighbor not in self._delivered and
                neighbor not in self._to_be_delivered and
                self._select(neighbor, parent) )

    def _get_neighbors(self, next_cell):
        if random.random() < self._randomness:
            return self._layout.random_neighbors(next_cell)
        return self._layout.neighbors(next_cell)

First thing I notice is that we should probably move the setting of _origin up to the top, as it is a parameter to the class creation.

class Flooder:
    def __init__(self, *, layout, origin):
        self._layout = layout
        self._origin = origin
        self._delivered = None
        ...

Commit: tidying.

I think we could indicate that available and in_any_room do not use the parent cell:

    def available(self):
        self.select(lambda cell, _parent: self._layout.is_available(cell))
        return self

    def in_any_room(self):
        self.select(lambda cell, _parent: self._layout.is_in_a_room(cell))
        return self

Those two are “convenience” methods for commonly-used conditions in select. We might wish to provide a convenience method for our latest use:

class Dungeon:
    def maker_flood(self):
        def can_traverse(new_cell, parent):
            if new_cell.room == parent.room:
                return True
            if new_cell.has_passage(parent):
                return True
            return False
        ...

Wow! Look at that Feature Envy. Let’s be having a Cell method for that. Which of these would we prefer?

    parent.can_move_to(new_cell)
    new_cell.accessible_from(parent)

The first one, I think.

As things stand, there is no test for the can_traverse feature and I have been confident enough in it, since it is so simple, to let that slide. But if we’re going to change it in a moment, we’d best have a test that we can watch break and heal up.

I start the test using in_any_room and it should fail with 1 not equaling 3.

    def test_passage(self):
        layout = DungeonLayout(10, 10)
        map = '''
        1122
        1122
        '''
        layout.add_rooms_from_map(map)
        layout.add_passage((Cell(1,1), Cell(2,1)))
        results = dict()
        for cell, distance in (Flooder(layout=layout, origin=Cell(1,0))
                                    .in_any_room()
                                    .flood()):
            results[cell] = distance
        assert results[Cell(2,0)] == 3

Expected :3
Actual   :1

Now let’s do can_traverse in Flooder, but refer to can_move_to in Cell.

    def can_traverse(self):
        self.select(lambda cell, parent: parent.can_move_to(cell))
        return self

And use it in the test:

        for cell, distance in (Flooder(layout=layout, origin=Cell(1,0))
                                    .can_traverse()
                                    .flood()):
            results[cell] = distance

Now I expect a failure to find can_move_to in Cell.

E   AttributeError: 'Cell' object has no attribute 'can_move_to'

And let’s be having it:

class Cell:
    def can_move_to(self, cell):
        return cell.room == self.room or self.has_passage(cell)

And the test passes. Now let’s fix the new Flooder to use the new can_traverse method:

class Dungeon:
    def maker_flood(self):
        def can_traverse(new_cell, parent):
            if new_cell.room == parent.room:
                return True
            if new_cell.has_passage(parent):
                return True
            return False
        def further(c, n):
            return n + 1
        if len(self.flood_list) > 0:
            self.flood_list = SpriteList()
            return
        for cell, distance in (Flooder(layout=self.layout, origin=self.player_cell)
                .in_any_room()
                .select(can_traverse)
                .next_value(further)
                .flood()):
            cx, cy = cell.center_position(cell_size)
            text = arcade.create_text_sprite(text=str(distance),font_size=8)
            text.center_x = cx
            text.center_y = cy
            self.flood_list.append(text)

I had forgotten that the default value return is manhattan distance, so we can remove that bit as well.

    def maker_flood(self):
        if len(self.flood_list) > 0:
            self.flood_list = SpriteList()
            return
        for cell, distance in (Flooder(layout=self.layout, origin=self.player_cell)
                .can_traverse()
                .flood()):
            cx, cy = cell.center_position(cell_size)
            text = arcade.create_text_sprite(text=str(distance),font_size=8)
            text.center_x = cx
            text.center_y = cy
            self.flood_list.append(text)

So that’s much nicer and still floods the screen with the walking distance from Dot for every cell on the screen.

Commit: implementing and using new can_traverse selector.

Reflection

See why I try to take a look at new code a day or two after I’ve written it? As lovely as it seems on that first day (or as scary as it is as I back away slowly) we can generally find things to improve in and near that code.

Some thoughts come to mind.

There are probably other convenience methods that would be, well, convenient. I’m willing to write them, but only if they are needed at least once. So I speculate that in_same_room might be useful, but we’ll wait and see.

The default for value = manhattan distance is probably a good one. I wonder what other value calculations we have ever done in earnest. A quick look tells me that, so far, it’s the only calculation we have done. We’ll see if anything else every comes along. I can imagine collecting things but I’m not sure what things we might collect.

The name sliding between can_traverse and can_move_to did not escape me. I think can_traverse better connotes that we are traveling far, and can_move_to connotes a single step. Perhaps can_step_to would be better.

More significant, in Cell, we have quite a bit of rigmarole tied up in moving:

class Cell:
    def has_passage(self, cell):
        return self.layout.has_passage(self, cell)

    def attempt_move(self, direction):
        return self._get_connection_in_direction(direction).move()

    def _get_connection_in_direction(self, direction):
        return self._get_connection_offset(direction.value)

    def _get_connection_offset(self, offset):
        return self._get_connection(self.offset_by(offset))

    def _get_connection(self, cell):
        if not cell:
            return NoConnection(self, cell)
        elif self.room == cell.room:
            return OpenConnection(self, cell)
        elif self.has_passage(cell):
            return OpenConnection(self, cell)
        else:
            return NoConnection(self, cell)

class NoConnection:
    def __init__(self, origin, target):
        self.origin = origin
        self.target = target

    def move(self):
        return self.origin

class OpenConnection:
    def __init__(self, origin, target):
        self.origin = origin
        self.target = target

    def move(self):
        return self.target

Can all that possibly really be necessary? I think it is there to support the way Dot interacts with cell content, since the content can refuse entry to her in real time. But it is sure weird. We have no reason to deal with it now but I’ll make a note of it, and maybe we’ll be back.

I’ve only been here a bit over an hour but the article is feeling long, so let me take one more quick look at Flooder itself and then we’ll probably close out.

OK, these:

class Flooder:
    def flood(self):
        self._to_be_delivered = {self._origin: self._initial_value}
        self._delivered = set()
        while self._to_be_delivered:
            current_cell, current_value = self._next_to_deliver()
            yield current_cell, current_value
            self._enqueue_relevant_neighbors(current_cell, current_value)

    def _next_to_deliver(self):
        current_cell = next(iter(self._to_be_delivered))
        current_value = self._to_be_delivered.pop(current_cell)
        self._delivered.add(current_cell)
        return current_cell, current_value

In the second method next_cell and next_value might make more sense. In the first, there’s kind of a magical transition. Before the yield the cells are next to be delivered. After the yield they are most recently delivered.

Another concern. The second method saves the cell in self._delivered. That is not correct until after the yield.

Let’s do a little revision.

    def flood(self):
        self._to_be_delivered = {self._origin: self._initial_value}
        self._delivered = set()
        while self._to_be_delivered:
            current_cell, current_value = self._next_to_deliver()
            yield current_cell, current_value
            self._delivered.add(current_cell)
            self._enqueue_relevant_neighbors(current_cell, current_value)

    def _next_to_deliver(self):
        next_cell = next(iter(self._to_be_delivered))
        next_value = self._to_be_delivered.pop(next_cell)
        return next_cell, next_value

I think that’s better. Tests all pass.

That next/pop construction in _next_to_deliver has always bugged me. Some research suggests that we could use an OrderedDictionary to good effect.

    def flood(self):
        self._to_be_delivered = OrderedDict([(self._origin, self._initial_value)])
        self._delivered = set()
        while self._to_be_delivered:
            current_cell, current_value = self._next_to_deliver()
            yield current_cell, current_value
            self._delivered.add(current_cell)
            self._enqueue_relevant_neighbors(current_cell, current_value)

    def _next_to_deliver(self):
        next_cell, next_value = self._to_be_delivered.popitem(last=False)
        return next_cell, next_value

Now I think we’ll be just fine if we inline that method.

    def flood(self):
        self._to_be_delivered = OrderedDict([(self._origin, self._initial_value)])
        self._delivered = set()
        while self._to_be_delivered:
            current_cell, current_value = self._to_be_delivered.popitem(last=False)
            yield current_cell, current_value
            self._delivered.add(current_cell)
            self._enqueue_relevant_neighbors(current_cell, current_value)

Honestly I prefer it without the inlining. Go back one step, but inline there:

    def flood(self):
        self._to_be_delivered = OrderedDict([(self._origin, self._initial_value)])
        self._delivered = set()
        while self._to_be_delivered:
            current_cell, current_value = self._next_to_deliver()
            yield current_cell, current_value
            self._delivered.add(current_cell)
            self._enqueue_relevant_neighbors(current_cell, current_value)

    def _next_to_deliver(self):
        return self._to_be_delivered.popitem(last=False)

I think that’s just a bit more expressive. And we are down to crumbs and nits, so let’s commit and sum up.

Summary

Of course there’s always something to improve, but here I think we have made some improvements of value. We’ve made the Flooder easier to use, we’ve added a test for its new capability, And we’ve simplified the code itself, by adding a very simple method to Cell. (We’ve also identified some code in Cell that could perhaps use a little improvement.)

A pleasant session. I wish you the same. See you next time!