Long Refactoring: Extract Helpers

As the refactoring process continues, we will continue to decompose the large central class and functions. Right now, the SudokuSolver class is performing two functions. It is holding and managing the list of the puzzles, and it is solving them.

The heart of this code is the function tree_to_solution_string. Right now, I can’t call that by itself, as the SudokuSolver creates a bunch of helper objects before running through the whole set of tests. how can we tease this apart?

We have two small helper objects in this code: the box_index_table and the board index table. Lets’ split them off the Solver, so we can move them around and reuse them as we may need. Start with the BoardIndexTable

class BoardIndexTable:
    def __init__(self):
        pass

Add this code below SudokuSolver. Why? because the functions that will make the methods of these classes are down there, and we want to make the code as easy to review as possible. We want to avoid moving code around too far. The change has been trivial, but run the tests just to be sure.

OK, time to make a real change. We are going to replace the variable in the SudokuSolver with an instance of our new class. However, in order to do that, we need to initialize the instance, which means calling the function fill_board_index_table. If we make that a non-member function, it gets de-indented, and the code review gets tougher. We are going to keep is as a member function, but of our new class. We can oly get away with this because it is the last function of the code. OTOH, that is why we chose it.

We need a member variable to hold the table. I call that table. I also changed the points that reference the table to look like this:

-                    index, self.board_index_table[index], curr_node)
+                    index, self.board_index.table[index], curr_node)

Pretty subtle. Some diff tools do a better job with minor changes on the line, but notice that the underscore before table became a dot. It keeps the lines the same length, and keeps the naming consistent. Here is the whole change.

diff --git a/treesudoku/tree_sudoku.py b/treesudoku/tree_sudoku.py
index f382971..0305598 100644
--- a/treesudoku/tree_sudoku.py
+++ b/treesudoku/tree_sudoku.py
@@ -29,7 +29,7 @@ class SudokuSolver:
         self.board_strings = board_strings
         self.boards_dict = self.strings_to_board_dict(self.board_strings)
         self.box_index_table = self.fill_box_index_table()
-        self.board_index_table = self.fill_board_index_table()
+        self.board_index = BoardIndexTable()
         self.solved_board_strings = dict()
         for key, value in self.boards_dict.items():
             return_string = self.tree_to_solution_string(value)
@@ -37,7 +37,7 @@ class SudokuSolver:
 
     def tree_to_solution_string(self, original_board):
         index = 0
-        head_node = Tree_Node(index, self.board_index_table[index])
+        head_node = Tree_Node(index, self.board_index.table[index])
         curr_node = head_node
         while index < MAX:
             curr_board_filling_node = head_node
@@ -58,7 +58,7 @@ class SudokuSolver:
                 if index >= MAX:
                     continue
                 new_node = Tree_Node(
-                    index, self.board_index_table[index], curr_node)
+                    index, self.board_index.table[index], curr_node)
 
                 row = int(new_node.board_spot[0])
                 col = int(new_node.board_spot[1])
@@ -174,6 +174,11 @@ class SudokuSolver:
             box_center[1] -= DIM
         return boxes
 
+
+class BoardIndexTable:
+    def __init__(self):
+        self.table = self.fill_board_index_table()
+
     def fill_board_index_table(self):
         return_list = []
         for row in range(DIM):

Commit to git. That keeps that change as a stand alone, reviewable chunk of code. A comparable change can be made with the BoardIndex. Note that we get luck: the value_valid also refers to the BoardIndex. Any other value it requires is passed in as a parameter. Let’s use that as part of our new class, too.

--- a/treesudoku/tree_sudoku.py
+++ b/treesudoku/tree_sudoku.py
@@ -28,7 +28,7 @@ class SudokuSolver:
     def __init__(self, board_strings):
         self.board_strings = board_strings
         self.boards_dict = self.strings_to_board_dict(self.board_strings)
-        self.box_index_table = self.fill_box_index_table()
+        self.box_index = BoxIndex()
         self.board_index = BoardIndexTable()
         self.solved_board_strings = dict()
         for key, value in self.boards_dict.items():
@@ -53,7 +53,7 @@ class SudokuSolver:
 
             curr_row = int(curr_node.board_spot[0])
             curr_col = int(curr_node.board_spot[1])
-            if self.value_valid(test_board, curr_row, curr_col):
+            if self.box_index.value_valid(test_board, curr_row, curr_col):
                 index += 1
                 if index &gt;= MAX:
                     continue
@@ -109,6 +109,11 @@ class SudokuSolver:
             if index1 == 8:
                 print('-' * 21)
 
+
+class BoxIndex:
+    def __init__(self):
+        self.table = self.fill_box_index_table()
+
     def value_valid(self, board, row_index, column_index):
         row = ['1', '2', '3', '4', '5', '6', '7', '8', '9']
         column = ['1', '2', '3', '4', '5', '6', '7', '8', '9']
@@ -130,8 +135,9 @@ class SudokuSolver:
             else:
                 return False
 
-        box_indexes = self.box_index_table[
-            self.find_box_of_index(str(row_index) + str(column_index))]
+        box_indexes = self.table[
+            self.find_box_of_index(
+                str(row_index) + str(column_index))]
         for index in box_indexes:
             row = int(index[0])
             column = int(index[1])
@@ -147,8 +153,8 @@ class SudokuSolver:
 
     def find_box_of_index(self, index):
         box = 'box not found'
-        for each_box in self.box_index_table:
-            if index in self.box_index_table[each_box]:
+        for each_box in self.table:
+            if index in self.table[each_box]:
                 box = each_box
                 break
         return box

Commit to git.

Each of these changes is a minimal, stand alone change. Each could be reviewed and merged into a larger project by someone not very familiar with the code base. This is how we make progress.

Leave a Reply

Your email address will not be published. Required fields are marked *

This site uses Akismet to reduce spam. Learn how your comment data is processed.