The Repo on GitHub

There is something I truly dislike about the current request protocol. Speaking of messages, should they be better organized? (Yes)

In addition to the items mentioned in the blurb, there are some “big” things that I have in mind working on, including additional objects in the World, and more sophisticated Bot behavior. In [Discovery; Future; Messages](/articles/-x024/biot/-br/l I listed pits and blocks in the world, smarter but still simple bots, more use of scents (pheromones), allowing the Bots to share world knowledge, and, of course, actually implementing client-server.

Of these, exactly none are of particular interest to me this morning. In the absence of the social nature that we originally imagined for this little project, and in the absence of some really exciting idea, I’m kind of losing steam.

One thought that I’ve had is to give the Bots finite energy reserves so that from time to time they have to find some kind of fuel so as to replenish their power. That would probably add some states to the state machine, which might be slightly amusing. We might, if we were lucky, even come up with a more interesting way of representing the machine. But not today.

Fortunately there are a couple of small things that we could do this morning.

The request language consists of a list of maps or dictionaries. Each map is a small set of key-value pairs, such as 'verb': 'turn', or 'direction': 'EAST'. Each individual action request amounts to a very short sentence in those pairs. And there is one keyboard that I think was a mistake.

Bots and Blocks and whatever objects exist in the world are identified by an integer value. Naturally, on both sides of the client-server line, the code on that side associates that identifier value with some kind of real object or data structure, but it’s the identifier that we pass to World to define which Bot we want to move, and the same identifier comes back from World, with a packet of values we can store in our client-side representation of the Bot. All pretty ordinary.

Because there are many potential kinds of things in the World, we call the world things “entities”. Fine, had to call them something. However, the little language of defined keys uses the word ‘entity’ as the key where the integer identifier is the value. That leads to potential confusion between entity as identifier and entity as the actual thing. The World refers to ‘entity_object’ for the actual entity and lets ‘entity’ stand for the identifier. This is … irritating.

I think we need new words. “identifier” seems too long. “id” is a Python keyword. Let’s use the word “key” for the identifier, and save the word “entity” for the actual data structure representing, um, the entity.

The “easy” part of this is probably changing the little language. The harder part, mostly just tedious, will be dealing with the code that uses the old terminology,

We actually have two uses of the identifier in the request language. We say ‘entity’ when we mean the Bot show action we are describing, and we say ‘holding’ when we refer to the object we want to drop. Let’s rename to ‘bot_key’ and ‘inventory_key’.

Feelings
I try to stay sensitive to how I feel when I program. It seems o me that when I feel relaxed and “good”, I do better work than when I am tense or worried. I am starting to feel nervous about the scope of change it may take to deal with this idea. There are strings, code, member names, error messages, … yikes!

I think I’ll try just breaking things to see what happens. I think there is really only one spot in World that truly relies on the word ‘entity’:

    def execute_actions(self, actions_list):
        for action in actions_list:
            if isinstance(action, dict):
                action_with_parameters = self.assign_parameters(**action)
                self.execute_action(**action_with_parameters)
            else:
                self._add_message(f'action must be dictionary {action}')

    def assign_parameters(self, entity=None, **parameters):
        if entity:
            self.ids_used.add(entity)
            parameters['entity_object'] = self.entity_from_id(entity)
        return parameters

The assign_parameters method strips out the key ‘entity’ from the action. If we change that variable name, it will change the requirement for every request everywhere to switch to using the word we choose. Let’s do it:

    def assign_parameters(self, bot_key=None, **parameters):
        if bot_key:
            self.ids_used.add(bot_key)
            parameters['entity_object'] = self.entity_from_id(bot_key)
        return parameters

Eleven tests break, and of course the Game will be broken as well. Let’s just search for ‘entity’ and see what we see. A scan of the window makes me believe that I can replace all of them. PyCharm will do that nicely. All tests are passing. Wow! Let’s see about the Game though. It just makes calls to World, asking it to add bots. It doesn’t issue any requests at all. And it runs fine.

Commit: change request key ‘entity’ to ‘bot_key’.

Now there is this thing, used only in World, that irritates me:

    def assign_parameters(self, bot_key=None, **parameters):
        if bot_key:
            self.ids_used.add(bot_key)
            parameters['entity_object'] = self.entity_from_id(bot_key)
        return parameters

    def execute_action(self, verb=None, entity_object=None, **details):
        if entity_object or verb == 'add_bot':
            match verb:
                case 'add_bot': self.add_bot_using(**details)
                case 'step': self.step(entity_object)
                case 'drop': self.drop_using(entity_object, **details)
                case 'take': self.take_forward(entity_object)
                case 'turn': self.turn_using(entity_object, **details)
                case 'NORTH' | 'EAST' | 'SOUTH' | 'WEST' as direction:
                    self.turn(entity_object, direction)
                case _:
                    self._add_message(f'Unknown action {verb=} {details=}')
        else:
            self._add_message(f'verb {verb} requires entity parameter {details}')

Now we can rename the parameter and the key to “entity”. Or to “bot”. Why not “bot”? It is a bot. There is no other case for that key. Change it to bot and fix two tests. I change the error message and the tests that check it, resulting in this:

    def assign_parameters(self, bot_key=None, **parameters):
        if bot_key:
            self.ids_used.add(bot_key)
            parameters['bot'] = self.entity_from_id(bot_key)
        return parameters

    def execute_action(self, verb=None, bot=None, **details):
        if bot or verb == 'add_bot':
            match verb:
                case 'add_bot': self.add_bot_using(**details)
                case 'step': self.step(bot)
                case 'drop': self.drop_using(bot, **details)
                case 'take': self.take_forward(bot)
                case 'turn': self.turn_using(bot, **details)
                case 'NORTH' | 'EAST' | 'SOUTH' | 'WEST' as direction:
                    self.turn(bot, direction)
                case _:
                    self._add_message(f'Unknown action {verb=} {details=}')
        else:
            self._add_message(f'verb {verb} requires bot_key parameter {details}')

Green. Commit: rename ‘entity_object’ World key to ‘bot’.

Now there are a number of methods referring to entity_object. I’ll search them out and rename to bot. Commit: rename parameters.

I cruise around a bit and discover Cohort referring to bot.id. And that leads us to Bot itself (this is client side, remember.)

class Bot:
    holding = Forwarder('_knowledge')
    id = Forwarder('_knowledge')
    location = Forwarder('_knowledge')
    new_direction_name = Forwarder('_knowledge')
    update = Forwarder('_knowledge')

We’d like to rename id to key. I think this one will be nasty. If this were Java or Kotlin or C#, our IDE would have a solid sense of whether we’re referring to a Bot and its id or not. Here … I’m not so sure.

I’ll try the rename. 39 tests fail. I rename the variable in Knowledge. Now it’s only … 38 tests failing. I think PyCharm didn’t know which calls to .id to fix.

    def _add_entity(self, entity):
        self.map.place(entity)
>       return entity.key
E       AttributeError: 'WorldEntity' object has no attribute 'key'

Ah … I think that the change partially leaked over to World. OK, so be it, we can always roll back if this gets ugly.

Changing the internal key in WorldEntity is harmless. Changing its external property from id to key has broken tests down to “only” 14.

    def take_conditionally_at(self, take_location, condition):
        item = self.at_xy(take_location.x, take_location.y)
        if item and condition(item):
>           self.remove(item.id)
E           AttributeError: 'WorldEntity' object has no attribute 'id'

Right, that’s going to want to say .key. Only 9 failing.

    def as_dictionary(self):
>       held = 0 if not self.holding else self.holding.id
E       AttributeError: 'WorldEntity' object has no attribute 'id'

Change that one. Only 3 tests failing.

    def test_cannot_set_into_knowledge(self):
        bot = Bot(10, 10)
        with pytest.raises(AttributeError) as error:
            bot.key = 101
        assert str(error.value) == "cannot set 'key'"

That one just needed ‘key’ there at the end.

    def __init__(self, bot=None):
        self.bots = {}
        if bot:
>           self.bots[bot.key] = bot
E           AttributeError: 'FakeBot' object has no attribute 'key'

Ah my test double.

class FakeBot:
    def __init__(self, key):
        self.key = key
        self.actions = []

And we are green. Commit: rename id to key throughout.

Reflection

OK, that was large. It only took about 20 minutes but we changed 13 files. In a large application, in Python, I think this might have been too broad a change to have done so casually. On the other hand, if the Bot side and World side were in separate source projects, the first renaming would not have bled over to the World side, and we would have had a much easier time of it.

If there is a lesson to be learned, it is that I “should” probably have addressed the id naming issue as soon as I became aware that Python uses it as a keyword, and, similarly, I “should” have noticed and dealt with the entity-entity_object naming issue when I first noticed it. Hindsight etc., don’t “should” all over yourself, etc., etc. But I would do well to slow down a bit and pay a bit more attention to these things.

Moving On …

I’ll do a bit of searching. I find one test file, TestPython, that is using old terms like ‘id’ and ‘entity_object’. It’s just a bunch of tests for how Python handles dictionaries in match/case statements. We don’t even use that technique any more. I do a bit of renaming for consistency. Nothing to see here.

In WorldEntity this statement is highlighted by PyCharm:

    def as_dictionary(self):
        held = 0 if not self.holding else self.holding.key
        return {...}
It underlines key and the message is: “Cannot find reference ‘key’ in ‘int EntityKind None’”

I’m not quite sure why PyCharm thinks the type of self.holding is one of those things but we can do some type hinting and help it out.

    @property
    def holding(self) -> Self:
        try:
            return self._dict['held_entity']
        except KeyError:
            return None

    @holding.setter
    def holding(self, value: Self):
        self._dict['held_entity'] = value

That’s Python type hinting indicating that the thing hinted is of the same class as we are presently defining. Why? Well, I reckon it’s because the class WorldEntity isn’t fully defined yet, so we can’t refer to it, but anyway, that’s how you do that.

Reflection: Type Hinting

I have a strong preference for “duck-typing” languages like Python or Ruby, coming from a fairly long and deep relationship with Smalltalk. There are things that are far easier to do in Python than in Java or Kotlin or whatever. And I do get tired of typing things like

SalaryRecord salaryRecord = SalaryRecord(...)

But you only need one situation where a type check by the compiler or IDE saves you from a subtle bug to appreciate what great value there is to a compiler and IDE that understand what kind of thing you’re talking to, not just what you’re saying to it. PyCharm does what it can to work out the likely types of things, as it did (incorrectly) in the example above, but it can only do so much in the absence of some clear markers in a few places.

I think that if I were condemned to work on a large Python project, I’d be inclined to treat type hints as “strongly recommended”, encouraging the team to use them liberally but not necessarily everywhere. I try to use names that communicate what my objects are, unlike a programmer I once knew who named their variables things like duck and cow and sheep, and we were implementing a database language, not a farm support app. But what I’ll try to do is a bit more type hinting. I’ll try to do enough to be useful but little enough so that it isn’t obtrusive into my thought process. Maybe I can find a nice balance, just enough hinting to get PyCharm to be more helpful, but little enough that it’s painless to do it.

As for duck typing … there are a few test doubles in the system, standing in for real objects. But overall, I can’t think of a place where we really use duck typing to provide two truly different objects that can be used willy-nilly in some context. Perhaps there are such places.

An interesting question: how could we determine whether or not there are duck-typing cases going on in this program? Offhand I have no idea how to answer that question, unless I were to remember, as I do with the Fake object I looked at a half hour ago. It’s a bit scary: we saw the effect with our test double.

Scary

The real Bot system was changed to send and accept keywhere it used to use id. But our test double did not. That turned up quickly in a test. But suppose we had two objects that wandered through the code. Two kinds of bots or something. And suppose we added and used a new method on one of them. How would we determine that there was a second kind of bot that may need that method?

Python does provide the Abstract Base Class capability, so it is possible to define the classes such that they must implement the same protocol. But even then, you have to be sure to put the new method up in the abstract class, not just in one or more concrete classes.

If there are duck-typed classes in the system, how can we know? If they need the same method name for a new method, how can we be sure that we’ve put that method everywhere it needs to be?

Can you see why I think this is scary? We could have a long-standing error waiting to happen, and it’s not hard to imagine a crash occurring in production, a long time and a long way away from the mistake. Today, it turned up quickly. Is there another such error in the code? I do not know … and I don’t even know how to find out for sure.

Scary!

Summary

We have improved the little language of requests, normalizing the names to better distinguish between things and the keys of the things. There is more we could do. We have the word holding, which, if I’m not mistaken, is the key of the object the bot is holding. We spoke above of renaming that to ‘inventory_key’ or something like that. But we haven’t done it yet.

We’ve had to change some messages both in the code and in corresponding tests. In my line of work we call that “duplication” and we are supposed to get rid of it, We need some kind of place to keep the messages. I’m not even sure whether an f-string can be used indirectly: we’ll have to find out.

And there is still an issue with JSON and the Location object. We’re passing back a Location and when we go through JSON, that will not work We should work on that.

So, the good news is that I’ve found some things to do to put off things that I don’t really want to do. That would get me fired on a real job, but here, so long as the things we work on teach us lessons, we’re good.

See you next time!