Sudoku: Smooth

After a review of notes, we move component looping from one class to another, in seven very smooth moves. I forgot to commit two of them, but it was still quite smooth.

Notes include:

• 10 Aug: Decide about Naked Pairs loop in SudokuTechniques. Should be in Naked Pairs?
• More puzzles from Internet?
• 06 Aug: hack checking for line size in SudokuTechniques / NakedPairs?
• 06 Aug: Really no NakedPairs in our 9x9 puzzle?
• 06 Aug: NakedPairs code bounces around a lot. [I think this is OK, but review is coming.]
• 06 Aug: Unused puzzle member in NakedPairsTechnique? [But looping may affect this.]

I think we’ll address the looping question. The issue is that the Hidden Pairs implementation has looping over components in the HiddenPairsTechnique class, while Naked Pairs has it in the SudokuTechniques class rather than NakedPairs class. I think the looping belongs in the specific technique, because otherwise, the caller has to know too much about the specific technique and how it works. So let’s fix that.

A quick check tells me that I forgot to commit yesterday. After reviewing the diff, commit: HiddenPairs installed with logging and new tests.

Now then, the code in question:

class SudokuTechniques:
    def apply(self):
        self.changed = True
        while self.changed:
            self.changed = False
            self.changed |= self.only_one_possible_value()
            self.changed |= self.only_one_possible_position()
            self.changed |= self.naked_pairs()
            self.changed |= self.hidden_pairs()
        return self.puzzle

    def hidden_pairs(self):
        if self.puzzle.line_size != 9:
            return False
        hp_technique = HiddenPairsTechnique(self.puzzle)
        return hp_technique.apply()

    def naked_pairs(self):
        if self.puzzle.line_size != 9:
            return False
        changed = False
        for component in self.puzzle.all_components():
            np = NakedPairsTechnique(self.puzzle, component)
            changed |= np.apply()
        return changed

We see the issue. The newer hidden_pairs method just creates the technique and lets it run. The older naked_pairs loops over all components, creating individual techniques and accumulating their results. This latter way is not cool.

I am concerned about the robustness of our tests here. Let’s quickly just comment out the works here and make sure that something fails.

Good news: test_naked_pairs_via_sudoku_techniques fails if this code doesn’t exist. So how might we proceed to move this behavior out of SudokuTechniques? Review the NakedPairsTechnique:

class NakedPairsTechnique:
    def __init__(self, puzzle, component):
        self.puzzle = puzzle
        self.component = component

    def apply(self):
        def cond(p1, p2):
            return len(p1) == 2 and p1 == p2

        changed = False
        for naked_pair in self.component.find_candidates_pairs(cond):
            changed |= self.component.remove_naked_pair(naked_pair)
        return changed

Lets begin by extracting everything in apply into a new method, apply_for_component.

class NakedPairsTechnique:
    def __init__(self, puzzle, component):
        self.puzzle = puzzle
        self.component = component

    def apply(self):
        return self.apply_for_component()

    def apply_for_component(self):
        def cond(p1, p2):
            return len(p1) == 2 and p1 == p2

        changed = False
        for naked_pair in self.component.find_candidates_pairs(cond):
            changed |= self.component.remove_naked_pair(naked_pair)
        return changed

This passes the tests. Let’s see how often we can commit, how small we can make our steps. Commit: refactoring toward handling all components.

Change the signature of the new method to accept a component parameter.

    def apply(self):
        return self.apply_for_component(self.component)

    def apply_for_component(self, component):
        def cond(p1, p2):
            return len(p1) == 2 and p1 == p2

        changed = False
        for naked_pair in self.component.find_candidates_pairs(cond):
            changed |= self.component.remove_naked_pair(naked_pair)
        return changed

Use the provided component:

    def apply_for_component(self, component):
        def cond(p1, p2):
            return len(p1) == 2 and p1 == p2

        changed = False
        for naked_pair in component.find_candidates_pairs(cond):
            changed |= component.remove_naked_pair(naked_pair)
        return changed

Darn! Forgot to commit the signature change. Commit now, same message.

Now we have an issue outside this class. We have users who think they have to provide the component. What would happen if we were to ignore it and just loop over all components in apply? Let’s at least try it.

    def apply(self):
        changed = False
        for component in self.puzzle.all_components():
            changed |= self.apply_for_component(component)
        return changed

What would happen is, all the tests would still pass. Do we have the nerve not to even look at them? I’m not sure. Prudence would say that we should at least examine them, and you know that she’s usually right about these things. But let’s finish up here. We now wish not to be passed a component in our init. Change Signature:

class NakedPairsTechnique:
    def __init__(self, puzzle):
        self.puzzle = puzzle

    def apply(self):
        changed = False
        for component in self.puzzle.all_components():
            changed |= self.apply_for_component(component)
        return changed

    def apply_for_component(self, component):
        def cond(p1, p2):
            return len(p1) == 2 and p1 == p2

        changed = False
        for naked_pair in component.find_candidates_pairs(cond):
            changed |= component.remove_naked_pair(naked_pair)
        return changed

Darn, I forgot to commit again! This tiny steps thing is hard! Commit: technique now loops over all components, accepts no component parameter.

Now let’s examine the references to the class, in prod and tests.

Hahaha, he laughed. Right off the bat, I find the thing we were here to change:

    def naked_pairs(self):
        if self.puzzle.line_size != 9:
            return False
        changed = False
        for component in self.puzzle.all_components():
            np = NakedPairsTechnique(self.puzzle)
            changed |= np.apply()
        return changed

Change it. (I had considered changing it first, and working By Intention, but decided not to so quickly that I didn’t even mention it.)

    def naked_pairs(self):
        if self.puzzle.line_size != 9:
            return False
        return NakedPairsTechnique(self.puzzle).apply()

Green. Commit: use NakedPairs properly in SudokuTechniques.

You’ll notice that I am committing all this despite not having inspected the tests. I am confident enough in the tests, and the rules surely are that if all the tests run, we can commit. Continuing with the review of tests:

    def test_naked_pairs(self):
        puzzle = Puzzle.empty()
        row = Component.row(puzzle, 0)
        row.set_candidates_at(0, ['1', '2'])
        row.set_candidates_at(4, ['1', '2'])
        assert row.candidates_at(0) == ['1', '2']
        technique = NakedPairsTechnique(puzzle)
        changed = technique.apply()
        assert changed is True
        new_row = Component.row(puzzle, 0)
        for cell in [1, 2, 3, 5, 6, 7, 8]:
            candidates = new_row.candidates_at(cell)
            assert candidates == ['3', '4', '5', '6', '7', '8', '9']

That looks custom made to be done the way it’s done. Formerly we’d have been passing in the row, now we don’t have to. Looks good to me. The other two tests are similar, looking just as you’d wish they look. I think we are happy as clams or perhaps even more happy.

We’ve committed, so let’s sum up.

Summary

In five commits, over less than a half hour, including writing this article, we have changed the system so that the NakedPairsTechnique takes care of the necessary looping, and the primary user, SudokuTechniques no longer has to do that. That’s the good news.

There were, I think, two moments when we could have committed and did not. My brother GeePaw Hill says that he commits on every green, and I believe him, while suspecting that even he forgets sometimes. I know that I do forget: you just saw me do it.

I could imagine a hook or plugin that would commit on every green. But that’s not what I want: I don’t want the commits to be automatic in the computer, I want tiny steps each with a commit, to be automatic in my mind. So what I really need is a plugin that rings a little chime and whispers “we could commit now, you tiny fool”. Seriously … I have PyCharm configured to run the tests whenever I stop typing. It does that, and it pops up a little alert in a vague shade of green if good and sort of red if not.

I get used to those going by and the colors are not bright enough to grab my attention. In any case, as I type and pause to think, I see a lot of perfectly reasonable reds and fewer greens, which I think is the nature of not typing the entire method into NotePad and only pasting it in when it is correct, which seems to me not to be the way to do it.

I guess I just have to keep working on the habit.¹

Reviewing the overall process, it went very smoothly, with the tests pretty much always green. I would certainly forgive myself, or anyone else, for only committing at the end of the process, but I would say, as kindly as I can, “Ron, you [REDACTED], you might consider locking in each of those good steps with a commit.”

I might reply that, sometimes, I’m not really sure that the step is a step forward, so I tend not to commit everything I do that passes, but only the steps that I’m pretty sure will not be rolled back. If I did reply that way, I might then advise myself to think about that, at which point I might reply that my Git skills are such that I’m not quite sure how to back up multiple steps and skip over some mistaken ones. Then I might suggest that it wouldn’t exactly hurt if I were more adept with Git.

So that was an interesting conversation. Quite possibly if I knew how to check out from the past, make a change, and put that back at HEAD, I would be more able to take and commit tiny steps. I’ll see if I can find out how to do that. I think I know some ways that won’t work.

Summary Summary

A very smooth set of steps to a design improvement. The code is more consistent and the clams all seem quite complacent.

See you next time!