The Repo on GitHub

Woot! I believe we have a complete disconnection between the client bot and the world. The world is running its own bot and I’m sure we have no cross-talk. I provide some general observations about this long refactoring, and its general implications.

Woot! Did I mention that?

This morning we had this:

class Bot:
    def perform_actions(self, actions):
        for action in actions:
            match action:
                case 'take':
                    self.world.take_forward(self)
                case 'drop':
                    self.world.drop_forward(self, self.inventory[0])
                case 'step':
                    self._old_location = self.location
                    self.world.step(self)
                case 'NORTH' | 'EAST' | 'SOUTH' | 'WEST':
                    self.world.set_direction(self, action)
                case _:
                    assert 0, f'no case {action}'

One case at a time, I did this:

    def perform_actions(self, actions):
        connection = DirectConnection(self.world)
        for action in actions:
            match action:
                case 'take':
                    connection.take(self)
                case 'drop':
                    connection.drop(self, self.inventory[0])
                case 'step':
                    self._old_location = self.location
                    connection.step(self)
                case 'NORTH' | 'EAST' | 'SOUTH' | 'WEST':
                    connection.set_direction(self, action)
                case _:
                    assert 0, f'no case {action}'

As you can see, we now run all the accesses to world though a DirectConnection, which now looks like this:

class DirectConnection:
    def __init__(self, world):
        self.world = world

    def add_bot(self, x, y):
        from bot import Bot
        id = self.world.add_world_bot(x, y)
        client_bot = Bot(x, y)
        client_bot.id = id
        client_bot.world = self.world
        result = self.world.fetch(id)
        client_bot._knowledge = copy.copy(result)
        return client_bot

    def set_direction(self, bot, direction_string):
        self.world.command('turn', bot.id, direction_string)
        result = self.world.fetch(bot.id)
        bot._knowledge = copy.copy(result)

    def step(self, bot):
        self.world.command('step', bot.id)
        result = self.world.fetch(bot.id)
        bot._knowledge = copy.copy(result)

    def take(self, client_bot):
        self.world.command('take', client_bot.id)
        result = self.world.fetch(client_bot.id)
        client_bot._knowledge = copy.copy(result)

    def drop(self, client_bot, block):
        self.world.command('drop', client_bot.id, block.id)
        result = self.world.fetch(client_bot.id)
        client_bot._knowledge = copy.copy(result)

When we add a bot, we make two, one for the World and one for the client. This will have to be unwound when we work on initializing, but in any case the two have different instances and different knowledge.

Each of the commands goes through World.command, passing only the input bot’s ID, so we know we do not have the client bot. This is command.

class World:
    def command(self, action, parm1, parm2=None):
        if action == 'step':
            bot = self.map.at_id(parm1)
            self.step(bot)
        elif action == 'take':
            bot = self.map.at_id(parm1)
            self.take_forward(bot)
        elif action == 'drop':
            bot = self.map.at_id(parm1)
            block = self.map.at_id(parm2)
            self.drop_forward(bot, block)
        elif action == 'turn':
            bot = self.map.at_id(parm1)
            self.set_direction(bot, parm2)
        else:
            raise Exception('Unknown command')

This could use a little improvement:

class World:
    def command(self, action, bot_id, parameter=None):
        bot = self.map.at_id(bot_id)
        if action == 'step':
            self.step(bot)
        elif action == 'take':
            self.take_forward(bot)
        elif action == 'drop':
            block = self.map.at_id(parameter)
            self.drop_forward(bot, block)
        elif action == 'turn':
            self.set_direction(bot, parameter)
        else:
            raise Exception('Unknown command')

And this, called only from th above:

class World:
    def set_direction(self, world_bot, direction_name):
        match direction_name:
            case 'NORTH':
                world_bot.direction = Direction.NORTH
            case 'EAST':
                world_bot.direction = Direction.EAST
            case 'WEST':
                world_bot.direction = Direction.WEST
            case 'SOUTH':
                world_bot.direction = Direction.SOUTH
            case _:
                pass

Everything else took place in the tests and only one had to be recast a bit, along the lines of the one this morning:

    def test_three_blocks_near(self):
        from block import Block

        def turn_move_and_update():
            client_bot.perform_actions(['WEST', 'step'])
            world.update_client_for_test(client_bot)
            actions = client_bot.update_for_state_machine()
            client_bot.perform_actions(actions)

        world = World(10, 10)
        world.add(Block(4, 4))
        world.add(Block(6, 6))
        world.add(Block(4, 5))
        client_bot = world.add_bot(6, 5)
        client_bot.direction_change_chance = 0.0
        client_bot.vision = None
        turn_move_and_update()
        assert client_bot.location == Location(5, 5)
        vision = client_bot.vision
        assert ('R', 5, 5) in vision
        assert ('B', 4, 5) in vision

Here, we get a free test of setting direction in our test for getting the right vision. It’s still quite a story test but the good news is that our tests are pretty likely to remain stable under change, or, if not to be quickly converted. I do think they’re still too complex and not fine-grained enough, but legacy tests are like that and we can deal with them as issues arise.

Commit: client-server split complete, runs through DirectConnection

Let’s sum up:

Callooh! Callay!

Er, I mean

Summary

Bottom Line
The Bot class is used in both world-side and client-side operations, but no instances ever cross the boundary. The world looks up its own copies when it is called, and when information is returned, it is copied, so that no back references occur. I am certain of this at above the 0.9 level.

This is a very good thing. We can be very confident that we are set up for any kind of client-server hookup we are aware of, and we have never had to build a bit of network code as yet. We’re ready, and not paying any network price until we want to

It was easy, this afternoon
This afternoon’s changes, converting to use a DirectConnection, went in smoothly, with only one test needing actual revision, so that it would make the right kind of calls rather than tampering with the innards of Bot.
“Make the change easy, then make the easy change”
There have been about seven articles moving toward this easy change. Some of them were pure experiments, and quite a few were slowly surrounding the problem, herding the code to the point of isolation in one method where we did this afternoon’s easy change.

(The felicitous quotation is from Kent Beck.)

Small steps are “always” possible
I would set the value of “always” to about 0.95. I have seen plenty of code that I wanted to rewrite, but I have never seen code whose design could not be brought into line with our needs by refactoring in small steps, keeping things running.

That is not to say that it is always faster to refactor, although I think that it usually is faster. The main reason why it’s faster, I believe, is that when we rewrite, we run into the same issues that gave us the design that needs improvement, and we aren’t that much better equipped to deal with them. In small-step refactoring, we face things one at a time and the issues are smaller.

Faced with code whose design I do not like, I will always give refactoring a strong vote.

Small steps can include new code
Refactoring in small steps doesn’t mean just moving things around and giving them better names. We will often create new objects and insert them at various levels in our refactored implementation. The DirectConnection is a clear example here, and before we’re really done I’m sure we’ll create some new objects and replace some old ones, on both sides of the line.

What we have done today is drawn the hard line between client-side and server-side code. That doesn’t mean that either side is ideal for its purpose: far from it, as we’ll surely see in upcoming articles. It’s just that now our objects will not have to serve two purposes, making them, with any luck and skill, smaller, simpler, easier to test.

Big mistake in the old design
Doubtless there were many mistakes. I think the worst mistake was the notion I had of the world calling back to objects, setting values while the call to world was still in effect. You’d ask to step and the world would set your location for you. At the time, I thought that was pretty neat. But it was a bear to untangle.

The obvious alternative, having world return result, would not have been much better. Either way, we would have an immediate return, and in the client-server mode, we need a deferred return. Without massive multi-threading, which would be seriously confusing, I don’t think we could have preserved either the call back or return form.

Smaller mistakes
Allowing the bot to change its own direction locally, and having that magically picked up in the world, was a mistake. I finally unwound that today. Perhaps doing it sooner would have been better.

There may be other mistakes that one could pick out with detailed review but the thing is this:

Mistakes are inevitable
No matter how much up front design we do, design mistakes will occur. We are only human, and we cannot foresee the future clearly enough to get it right. Coulda woulda shoulda thinking is a waste of time.
We can always fix mistakes
Especially with today’s refactoring tools, fixing design mistakes is pretty easy. The tools do most of the grunt work. But even without power tools, we can move what we have to, insert what we have to. The tool doesn’t do anything at all that we could not do manually. They relieve tedium, they don’t work magic.
Fixing early pays off
If I have any regrets, they’re mostly that I wish I had done something about an issue sooner than I did. I got so interested in Bot behavior that I tolerated things like the odd handling of direction, and I sort of think that dealing with it sooner might have been better.

Certainly if we find ourselves writing repetitive code or “working around” some issue, we’d do well to address the real issue directly. I think the main issue where that bits me here is in the tests, which are complicated, involve too many objects, and have needed small adjustments often.

Not all problems are solved
I am not sure even now what to do about the long tests. The fundamental operating principle of the program has separation between the world and the bot, and the results are in essence collaborative. Without mock objects, I’ve not yet found good ways to change a question like “if the bot hits the wall does it changed direction” into smaller more granular checks. I’m hopeful, and a bit optimistic, that the better separation of objects will get us there.

Perhaps we’ll be able to test first that the world sets knowledge correctly, and second, that the bot responds to the knowledge correctly. That second part will need to accommodate the two-phase character of the client-server cycle.

Net Net Net

This is a very good point in the development of this little program. It has, I am quite sure, enabled a client-server split, without ever having to actually do a network connection. I am proud of that, because every client-server effort I’ve ever seen seemed to set up the c-s stuff early and then suffer with it thereafter.

The separation makes design improvements on both sides much simpler, and they’ll be independent except when we devise new world capabilities, which is about as good as it can get. I’m sure we’ll want to come up with a better “command language” than the current string plus parameter scheme, and I think we may have a few more conversions back and forth than we need, especially in the direction-setting. But we’ll see. Now we can see, much more clearly.

It has been a long cycle, but all the way the changes were small and the system kept running throughout. No one went away and went heads-down and then came back with an impossible merge.

In short: Woot! See you next time!