Review: The Process

The Repo on GitHub

I do like the result of my changes to the state machine. I didn’t always feel good getting from there to here. Let’s see what I might possibly have done better. My conclusion surprises me.¹

Alone or Together

As GeePaw Hill puts it, we are in the business of changing code, and over the past 8 or so articles, I’ve been changing the Bot’s state machine from an inline collection of methods to a separate trio of tiny classes, one for each state. I had some expert help during the parts that went well, either via ideas or direct pairing. In the parts that did not go well, I worked alone and I think the first thing to learn was that things go better with help.

One of the most interesting aspects of the help I got from GeePaw was that he would ask questions about things tangential to what we were doing, raising a concern about something I might not be considering. I would say that most times, I was either considering it directly, or, more commonly, ignoring it but harmlessly, because I mostly know how the code works.

But once in a while, maybe one or two out of five questions, he’d call my attention to something that I had forgotten to consider, or that I had wrong. Those questions were worth their weight in gold, as they say, especially given that questions are generally of zero mass. They were worth a lot more than zero, because they prevented errors.

And the questions did not bother me. I never felt that he was contradicting me or trying to get me to change what I was doing. They were just questions and I was happy to answer them. That level of conversation during programming, essentially on topic, doesn’t bother me at all.

I could be a better pair.

I think that I am not good at that. I think that my interactions probably seem more like actual interruptions or attempted corrections to my pair. I think I need to work on that: I’ve been working alone too much over the past few years.

But there was more than that. There were pauses where we talked about what was needed. There were observations about the code that led to small improvements. I recall two situations in particular.

At one point, GeePaw asked me about the tired property of the Bot, where after it makes a hard decision to pick something up or put it down, it just kind of wanders around a bit. We put that in so that it wouldn’t just stand in one place picking up and putting down. GeePaw asked, I answered, and also said “That’s outside of Knowledge and it should be inside”. I knew that it needed doing, and that I hadn’t done it, and the question reminded me of that issue. And it turns out that that issue didn’t break anything, but it did make the process much more difficult, because instead of being able to refer just to the Knowledge, the state machine also had to refer to the Bot itself. That slowed us down, because part of what makes the final result nice is that everything the machine needs to know is in Knowledge.

At another point, we reflected on a method name that wasn’t as clear as it could be. We were right in the middle of some change sequence, with two or three balls in the air. I said it needed changing. I almost changed it: PyCharm would have done it just fine. GeePaw agreed. Then I said “No, it’s too much”, meaning that my brain didn’t have room for the change. He said “OK” and we completed the cycle.

Sharing ideas is fun!

The interplay of ideas is amazingly powerful and, between GeePaw and me, it’s generally great fun. We seem to play off of each other just about right. I have notes on my desk to try to help me to pair that well with whoever I’m pairing with. I’d like to do more and I need to bring my game up to be better at it.

There’s another advantage to working together that, on a real team with a real project, is quite important: more than one person is familiar with that code now. That gives the team more options in the next days, if they choose to work outside a mob situation, because there are more combinations of people who can work on the state machine. If only one person can work on a thing, and they have to work on one of their things, no progress can be made on the other.

I have no real experience with the “mob programming” style, and I have questions about its effectiveness short and longer term, and so on. But only experience can answer those questions, and I suspect that the answers will be different for every team.

Alone or together? I think together is better, in almost every case. And I only say “almost” to avoid the possibility that pairing with a honey badger might be a bad idea.

Chronology What Happened, When?

September 8 & 9

September 8th and 9th, where I moved to the Method Object, went pretty well and I felt pretty good about them. On the 10th, I started dealing with an important issue, namely that in the client-server mode, we do not get immediate feedback on the result of a Bot action. The bot tries to pick something up, and that will require an exchange with the server, and we are not prepared to commit to an actual network operation on every move every individual bot makes.

The upshot is that we have to first update, based on the Bot’s current status which could change our state, and then take action based on the new state and current status.

September 10

In the course of that change, the September 10th article makes clear that I did not see easy steps from where we were to where we had to go. I even considered setting back a long way in the past. I think I was actually considering going back to before the method object existed. That seemed too far and I didn’t go back, I kept pushing forward, though confused about what should be done.

I broke something and had to roll back. This is not necessarily a bad thing: when we are sufficiently confused, rolling back is a very strong move, although it is always a hard one to choose, because it feels like failure. Yeah, sure, but you know that feeling you get when you push your rook and then see what a bad idea it was and want to take it back? In programming, you can take it back.

Things went well for a while, then a mistake and roll back, didn’t hurt so much because I had already done one and it hadn’t killed me.

At the end of the day, I had lots of commits and felt pretty good … but that the overall effort was taking too long. The article itself was also long, but nothing really awful happened.

September 11

The big tell here comes in two parts: a long time between commits, and a general feeling that things were ragged. In particular, I found tests that were not helping, in particular not failing when “they should” have, because they were not testing the changes I was making, but still using the old code in places where they should not.

A wiser man than I would have noticed long before that the tests for bot state transitions and actions were messy, hard to write, and both oddly sensitive to things that didn’t matter, and sometimes completely unaware of things that did matter.

In that article I reported that while things seemed to be improving, I still felt that it was taking too long, and that it felt ragged. Even in review, however, I don’t see much that I could have done differently. Better ideas would always help, but there was no place where I seemed to really be off in the weeds for long at all.

Probably the main issue was increasing dissatisfaction with the tests.

September 12

I put in a good idea, the trick GeePaw suggested of returning a three-tuple that represents either the two methods to call from the method object or the new class to use from Class Per State. That triple allowed us to create a single class at a time and switch over to it. It was a bit tricky to get it set up and I rolled back twice in the process, but it went well.

GeePaw joined me for the afternoon and we did a couple more hours. We only committed once in that whole session, at the end.

We had something like three common errors that plagued us right along. The biggest was that we did not start writing tests for our new classes: we relied on the existing tests. In his defense, GeePaw didn’t write the old tests and we didn’t review them as a whole, but they were crummy, hard to write, hard to change, and often misleading.

Smaller errors included not moving tired into Knowledge, and a number of copy-pasta transcription errors that made the tests pass (because we didn’t check every state transition and because the tests weren’t that good). The game did not work. At least once we fixed the problem without creating a test to show the problem. We were resisting the tests because they were not serving us.

I fancy that had we been less tired by then, we’d have done some small tests. I did do them the next day.

September 13 & 14

I started with very nice tests for the existing two state classes, and then test-drove the last class. It all worked very nicely. Once the three classes were in place, I was able to refactor, in very small steps, until probably half the code was just gone, as things fell into their proper place.

As it stands, I think the class per state implementation is very nice. although, like all things, it could still use a little improvement.

Things I Did Wrong Areas for Improvement

I am fascinated to look back over the past articles and not see places where I made some mistake or process or code that is clearly where it all went wrong. There were times when it felt ragged, but I seemed never to have much of a notion of why. I have a few notions about what I should have seen.

The code we started with “just grew”. It was a brand new project idea with no real spec, and we just put things together to see what would happen. Then … we kept going. So despite how small it was (and is), the project had a lot of “legacy code” for its small size, objects that were incomplete, objects with more than one or even two ideas in them, places in the code where only part of our ideas were expressed and others left implicit … and some really very difficult “story tests” rather than tests for individual components each with a well-defined purpose.

Given that that was the case, we might have expected some raggedness as we clean up the campground, which apparently we had a party last night and kind of trashed the place. Setting expectations that we were going to encounter some nasty bits might have left us more comfortable when things got a bit messy.

The tests, though, really needed work. Working alone, I quickly came to rely on them, perhaps a bit too much, given their fragility, but also to avoid changing them because they were a pain to write and change. There are a few points in the history where the tests clearly let us down, though I kind of waved my hands at the issue at the time.

We can certainly see the difference between a new test:

    def test_walking_not_tired_without_block_goes_looking(self):
        state = Walking()
        knowledge = FakeKnowledge(tired=0, has_block=False)
        c = state.update(knowledge)
        assert isinstance(c, Looking)

And the older style:

    def test_can_drop(self):
        location = Location(10, 10)
        direction = Direction.NORTH
        knowledge = Knowledge(location, direction)
        knowledge.vision = [('B', 10, 9)]
        assert not knowledge.can_drop
        knowledge.vision = [('B', 9, 9)]
        assert knowledge.can_drop
        knowledge.vision = [('B', 11, 9)]
        assert knowledge.can_drop

    def test_looking_goes_to_walking_then_laden_if_block_in_inventory(self):
        bot = Bot(5, 5)
        vision_list = [('R', 5, 5)]
        bot.vision = vision_list
        machine = Machine(bot._knowledge)
        machine.set_states(machine.looking_states())
        assert not machine._knowledge.has_block
        machine.state(bot._knowledge)
        assert machine._action == machine.looking_action
        bot.receive(Block(2, 2))
        assert machine._knowledge.has_block
        machine.state(bot._knowledge)
        assert machine._action == machine.walking_action
        machine.tired = 0
        machine.state(bot._knowledge)
        assert machine._action == machine.laden_action

Test Doubles

The test doubles, especially the FakeKnowledge object, allowed much easier test setup. Because the state machine relies so strongly on the Knowledge object, which is updated during many of the operations, using a test double allowed tests to check individual fine-grain decisions without first moving everyone into the right position. That paid off immensely, and it is a trick that I rarely use.

Layers of Abstraction

We now have a number of separate objects with separate responsibilities at varying layers of abstraction: the Bot has Knowledge that it shares with the state machine, while not sharing its own local concerns. The Knowledge hides how it gets answers, while providing the answers that the state machine really wants. Inside the Knowledge there is a Vision, which handles recognizing patterns around the Bot and supports decisions, which are relayed through the Knowledge.

All these things were tangled together when we started, making it hard to see what was going on.

It is possible that more work refactoring the original Bot, providing better separation of concerns, would have been a better starting set of moves. We interspersed moves to centralize information with other moves, perhaps to our detriment.

Following Through

In at least one case that I can identify, I did not follow through with something that needed doing, and it came back to bite me. The notion of tired belongs in the shared Knowledge between bot and its states, and I knew that, and I did not move it in. That fact caused tests to fail or succeed mistakenly, and if I am not mistaken, I tried a clever patch to make it seem to be included, which seemed to work but later caused more trouble. Just moving it in earlier would have been better. Even now, I think it can be improved a bit, but it is at least in the right place, I believe.

Not following through on a design idea is what creates code that does not have clear separation of concerns. It’s easy to skip that follow-through, when one is hot on the trail of some feature. But over time, the cruft grows and then for a few days things feel really ragged.

Missed Abstraction

In the tests, at one point, there was code, something like this before a state object was created:

update, action, state = machine.state()
assert update == Machine.walking_update
assert action == Machine.walking_action
assert state == None

And then after it was created, the test needed to be changed to something like this:

update, action, state = machine.state()
assert update == None
assert action == None
assert isinstance(state, Walking)

That was tedious, error-prone, and generally not fun at all. I think a better test fixture, or at least some kind of “AssertionObject” would have made that process go much more smoothly. We generally only had to change a few at a time, so it never seemed to get quite painful enough, but honestly I think the tests would have been more expressive and easier to change.

Would it have been worth it? Perhaps. Did it make much difference? Probably not.

Smoothing Out the Raggedness

Overall, looking back, I don’t think we could have cut the time in half by some different set of steps starting on September 8th. And I don’t even think we “should” have done something prior to the 8th so that the code would have been better on that day.

What I think would have made things feel better would have been to have focused more, especially in the beginning days, on identifying concerns to be separated out, and making places for them to go. And certainly we can imagine we would have benefited from improving the tests, whose structure clearly indicates that the code, going in, was not very testable.

But I am not saying that we would have needed less overall time to get to where we are now. Possibly one less session? It’s hard to say. We can only get ideas as quickly as we get them, we only recognize issues when we recognize them.

The main thing that I think I missed was that I didn’t stop to reflect right when things started to feel ragged. Instead, I just plugged away until something worked: I was too focused on the immediate target. I think it’s probably better, when things start feeling rough, to sit back and think why, and then take a little time to go after the cause of the roughness.

In this case, I don’t think we’d have saved much time … but we would have saved some stress, and would probably have gained more satisfaction every day, instead of having a few days where we felt kind of badly.

Sometimes, we really do need to sweep up some scraps before we go back to building.

I’m surprised to find that I don’t see any serious mistakes in the past week, even though it often felt ragged. I think my mistakes were small in effect on the code, but larger in their effect on me. I think they were mostly in not cleaning up cruft quite as soon as I might have. I think the result would be about the same in code and time … and I think I would have felt better about it every day along the way.

Would you like to feel better every day you program? Maybe there are some small adjustments you can make to accomplish that.

See you next time!