The Repo on GitHub

This morning, I plan to start moving more directly toward one class per state. I’ll be borrowing an idea from GeePaw Hill. Any mistakes in what follows are his and his alone.

GeePaw’s idea, translated through my brain, for converting from a single class with unbound methods (lambdas) as states, to multiple classes, each one representing a state, goes like this:

Return from each state transition, a tuple, consisting of the unbound method(s) to be executed, and an optional class instance. Things start off returning the unbound methods, and no class. In the dispatch, check to see if the tuple has the class instance element. If it does, call the standard machine methods on that object. If not, call the unbound methods in the other part of the tuple.

Proceed by breaking out one class at a time, and change the tuples returned when that state is desired to have the corresponding class instance. The dispatch code will automatically switch over to the new class instance. Rinse, repeat.

When all classes are done, sweep through, cleaning up the tuple, just returning the class instances throughout.

I have the feeling that that last step is a bit large, if we actually return a structure and then stop, because we’ll want to unwind the structure everywhere. In our situation it won’t be much: we only have three states, which is why this is a good time to get this change in.

Let’s begin by scanning the existing Machine class and its use.

class Bot:
    def do_something(self):
        self.update()
        actions = self.state.state(self._knowledge)
        for action in actions:
            match action:
                case 'take':
                    self.world.take_forward(self)
                case 'drop':
                    self.world.drop_forward(self, self.inventory[0])
                case _:
                    assert 0, f'no case {action}'
        self.move()

    def update(self):
        pass

The machine returns actions, which we currently execute, but which we will pass to the server in some magical form at some future time. The update method is a placeholder for a suspicion I have, that we’ll need to update status because the response from the server comes too late to update in this do_something cycle. We’ll see.

class Machine:
    def __init__(self, knowledge):
        assert isinstance(knowledge, Knowledge)
        self._knowledge = knowledge
        self.tired = 10
        self._update = self.walking_update
        self._action = self.walking_action

    def state(self, knowledge):
        assert isinstance(knowledge, Knowledge)
        self._knowledge = knowledge
        self.tired -= 1
        self._update()
        return self._action()

    def walking_states(self):
        return self.walking_update, self.walking_action

    def looking_states(self):
        return self.looking_update, self.looking_action

    def laden_states(self):
        return self.laden_update, self.laden_action

    def set_states(self, states):
        self._update, self._action = states

    def walking_update(self):
        if self.tired <= 0:
            if self._knowledge.has_block:
                self.set_states(self.laden_states())
            else:
                self.set_states(self.looking_states())

    def walking_action(self):
        return []

    def looking_update(self):
        if self._knowledge.has_block:
            self.tired = 5
            self.set_states(self.walking_states())

    def looking_action(self):
        if self._knowledge.can_take:
            return ['take']
        return []

    def laden_update(self):
        if not self._knowledge.has_block:
            self.tired = 5
            self.set_states(self.walking_states())

    def laden_action(self):
        if self.tired <= 0:
            if self._knowledge.can_drop:
                return ['drop']
        return []

The machine has a two-phase operation in its state command, update and action. The member variables _update and _action contain method names in the class, one pair of names for each machine state, walking, looking, and laden.

The update operation assesses the situation and selects a new state. It is done first because the necessary information comes back to us after the action is done, in the next call to the machine via do_something. (Yes, this is a bit hard to think about.) The update sets the two members _update and _action to the new state.

Then the action takes place. Note that it is often the action from a different state, because update may have changed our state based on the response to the previous cycle. the action returns a list of things to do, which, for now, is just a list of strings representing commands.

According to the idea from GeePaw, we’ll add a third variable to the Machine, representing a class instance that embodies just one state.

I think that when we get done, our Machine class will be able to be very simple and there will be three new classes for the three states. I also suspect that we’ll be able to eliminate Machine entirely and move the logic up to Bot, but we’ll see about that when we get there.

The Machine presently calls set_states with a two-tuple, such as:

    def laden_update(self):
        if not self._knowledge.has_block:
            self.tired = 5
            self.set_states(self.walking_states())

    def walking_states(self):
        return self.walking_update, self.walking_action

    def set_states(self, states):
        self._update, self._action = states

Let’s first change all of these to return three items, and to store the third item, which will, in the fullness of time, be the state instance.

class Machine:
    def __init__(self, knowledge):
        assert isinstance(knowledge, Knowledge)
        self._knowledge = knowledge
        self.tired = 10
        self._update = self.walking_update
        self._action = self.walking_action
        self._state = None

    def set_states(self, states):
        self._update, self._action, self._state = states

This breaks many tests, because we are not supplying a state yet, but now we change each of the state-computing members:

    def walking_states(self):
        return self.walking_update, self.walking_action, None

    def looking_states(self):
        return self.looking_update, self.looking_action, None

    def laden_states(self):
        return self.laden_update, self.laden_action, None

We are green. Commit: states lists now include None for the new state instance not yet provided.

Now we need to check for the _state and use it. This will have to be done twice:

    def state(self, knowledge):
        assert isinstance(knowledge, Knowledge)
        self._knowledge = knowledge
        self.tired -= 1
        if self._state:
            pass
        else:
            self._update()
        if self._state:
            pass
        else:
            return self._action()

Green. Shall we commit? No, let’s replace the passes.

I’m glad I said that. This code troubles me:

    def state(self, knowledge):
        assert isinstance(knowledge, Knowledge)
        self._knowledge = knowledge
        self.tired -= 1
        if self._state:
            self._state.update()
        else:
            self._update()
        if self._state:
            self._state.action()
        else:
            return self._action()

As things stand, the Machine’s set_states method resets the three variables of our “tuple”, behind the scenes, when we call _update. Our new classes will have to be able to do that, but they do not have access to Machine. So we need to switch from set_state to returning the new state information directly.

Let’s first commit this much, as it is solid as far as it goes. Commit: calling update and action on _state instance if present.

Now let’s see if we can return the three things directly.

    def state(self, knowledge):
        assert isinstance(knowledge, Knowledge)
        self._knowledge = knowledge
        self.tired -= 1
        if self._state:
            self._state.update()
        else:
           self._update, self._action, self._state =  self._update()
        if self._state:
            self._state.action()
        else:
            return self._action()

This breaks all my tests, but it’s no surprise. I think I just have to change all these guys to return that three-tuple thing.

Two attempts fail. The first time, I just don’t get the return right. The second time, I thought I had done it right but in fact it broke a test. (It also breaks some state machine tests but that’s no surprise: they use the set_state method.)

I have rolled back twice. Lets go in smaller steps. First this:

    def state(self, knowledge):
        assert isinstance(knowledge, Knowledge)
        self._knowledge = knowledge
        self.tired -= 1
        if self._state:
            self._state.update()
        else:
            info = self._update()
            if info:
                self._update, self._action, self._state = info
        if self._state:
            self._state.action()
        else:
            return self._action()

Now if update returns anything, we unpack it into our state variables. So far, no one returns anything. We’re green.

Change this:

    def looking_update(self):
        if self._knowledge.has_block:
            self.tired = 5
            self.set_states(self.walking_states())

To this:

    def looking_update(self):
        if self._knowledge.has_block:
            self.tired = 5
            return self.walking_states()

We are still green. I want to check the game. It is still working. Change one more:

    def laden_update(self):
        if not self._knowledge.has_block:
            self.tired = 5
            return self.walking_states()

Still green. One more:

    def walking_update(self):
        if self.tired <= 0:
            if self._knowledge.has_block:
                return self.laden_states()
            else:
                self.set_states(self.looking_states())

Still green. One more:

    def walking_update(self):
        if self.tired <= 0:
            if self._knowledge.has_block:
                return self.laden_states()
            else:
                return self.looking_states()

Still green, tests still working, game still working. I wonder about senders of the set method. Let’s see those tests.

    def test_laden_goes_to_walking_if_no_block_in_inventory(self):
        bot = Bot(5, 5)
        machine = Machine(bot._knowledge)
        machine.set_states(machine.laden_states())
        machine.state(bot._knowledge)
        assert machine._action == machine.walking_action

These are all setting up initial states in the machine. They are all going to break when we start breaking out classes, but we’ll sort them out when the time comes.

Commit this: update methods return three elements, update, action, state rather than use set_state.

Now we can remove the check for info.

No we cannot and the reason is clear: we must ALWAYS return state info, not just when it changes. We have to do like this:

    def walking_update(self):
        if self.tired <= 0:
            if self._knowledge.has_block:
                return self.laden_states()
            else:
                return self.looking_states()
        return self.walking_states()

I do that for the other two:

    def looking_update(self):
        if self._knowledge.has_block:
            self.tired = 5
            return self.walking_states()
        else:
            return self.looking_states()

    def laden_update(self):
        if not self._knowledge.has_block:
            self.tired = 5
            return self.walking_states()
        else:
            return self.laden_states()

Green. Try removing the check now? We could but we’ve uncovered an interesting question.

If you are in a state and update wants to remain in that state, are you required to return the old state anyway?

Yes, I think that’s the rule. So we remove the check, which will result in an exception if anyone forgets.

    def state(self, knowledge):
        assert isinstance(knowledge, Knowledge)
        self._knowledge = knowledge
        self.tired -= 1
        if self._state:
            self._state.update()
        else:
            info = self._update()
            self._update, self._action, self._state = info
        if self._state:
            self._state.action()
        else:
            return self._action()

Commit: all states now return state-change info in all cases.

Pause

I took a break at this point and waited for GeePaw to become available. We started again at about noon. We worked until nearly two PM, and in that time had only one commit at the end. This is not impressive performance for us. We like to be able to commit about every twenty minutes or more often.

Long story short, with some remarks to follow, we got two state classes done and all the tests green. The Machine and its related classes, Looking and Laden are now like this:

from knowledge import Knowledge


class Looking:
    def update(self, machine, knowledge):
        if knowledge.has_block:
            knowledge.tired = 5
            return machine.walking_update, machine.walking_action, None
        else:
            return None, None, Looking()

    def action(self, knowledge):
        if knowledge.can_take:
            return ['take']
        return []


class Laden:
    def update(self, machine, knowledge):
        if not knowledge.has_block:
            knowledge.tired = 5
            return machine.walking_update, machine.walking_action, None
        else:
            return None, None, Looking()

    def action(self, knowledge):
        if knowledge.tired <= 0:
            if knowledge.can_drop:
                return ['drop']
        return []


class Machine:
    def __init__(self, knowledge):
        assert isinstance(knowledge, Knowledge)
        self._knowledge = knowledge
        self._update = self.walking_update
        self._action = self.walking_action
        self._state = None

    def state(self, knowledge):
        assert isinstance(knowledge, Knowledge)
        self._knowledge = knowledge
        knowledge.tired -= 1
        if self._state:
            info = self._state.update(self, self._knowledge)
            self._update, self._action, self._state = info
        else:
            info = self._update()
            self._update, self._action, self._state = info
        if self._state:
            return self._state.action(self._knowledge)
        else:
            return self._action()

    def walking_states(self):
        return self.walking_update, self.walking_action, None

    def set_states(self, states):
        self._update, self._action, self._state = states

    def walking_update(self):
        if self._knowledge.tired <= 0:
            if self._knowledge.has_block:
                return None, None, Laden()
            else:
                return None, None, Looking()
        return self.walking_states()

    def walking_action(self):
        return []

There are a few leftover methods in the Machine class, which I’m not showing because there are still out of date tests looking at them. This is a good picture of where we are.

The two state classes Looking and Laden handle all the actual operation of the Bots in those two states, with Walking yet to be done. After that, the Machine class should reduce to nothing but the init and a very simplified state method, and we believe that it will perhaps collapse further, on up into Bot. We’ll see what we think when we get there.

The “trick” with it all was to have three variables, _update, _action, and _state, which can be in two states, as I discussed at the top of the article, either two methods in the first two, or a class instance in the second. So we set them to (method, method, None) or (None, None, instance) depending on whether we’re going back to the main Machine, or new state classes.

Reflection

It really all went pretty well, but we made one serious but almost unavoidable error at the beginning: we did not move tired into Knowledge. I knew that it needed to go in there, but thought it could wait. I was mistaken, and both tests and logic broke until we finally pushed it into Knowledge and then got enough code referencing it there instead of in Bot, where it had been before.

Despite being careful, we had a few transcription errors, and they were often hard to spot. Python bit us at least once, because external code can say something like bot.tired=5 instead of bot._knowledge.tired=5 and it just happily jams a new tired member into the bot without a by your leave. We still have some tests that are doing that and we’ll have to root them out.

Perhaps our biggest mistake was in not writing small tests for each state class before writing it. We relied on the existing tests to find issues and then, when tests started breaking, we patched them, often poorly, and even let some fail for a while, even though in fact they were trying to tell us things.

The overall effect wasn’t bad, and our two little classes were nearly fine, but undetected transcription errors, patching or ignoring broken tests, and failing to write new ones slowed us down. We could have committed a time or two but I didn’t want to for some unspecified reason. Probably we should have stopped and explored why I wasn’t ready, and either committed, which would have been harmless, or uncovered my discomfort and fixed the issue, which might have been better.

Summary

So, bottom line … these classes are, we think, righteous, and the Walking class should be straightforward as well. But our process just didn’t provide small steps from green to green. We got it done … but it was rather ragged.

Next time, fail better. See you then!