Long Refactoring: Extract Method

This refactoring is my bread and butter. Functions tend to grow. Eventually, you need to split them. Find a section of the method that has its own self-containerd reason- for existence, and make that its own function.

I have in the back of my head that I want to extract a class that abstracts the boards from this code. I’ve been resisting the urge thus far, as keeping the board as a 2D array of cells is really conducive to sharing with other implementations of this code. However, the following refactoring should work to support either direction: pull out the code that constructs a board from the string. This has the added benefit of making it possible to write a unit test that calls tree_to_solution_string without having to parse all of the strings.

I do this refactoring in two steps. The first step I create an inner function and move the code into there. Call the function from the original location.

diff --git a/treesudoku/tree_sudoku.py b/treesudoku/tree_sudoku.py
index 7c4004e..6a8cc15 100644
--- a/treesudoku/tree_sudoku.py
+++ b/treesudoku/tree_sudoku.py
@@ -86,15 +86,19 @@ class SudokuSolver:
         return return_string
 
     def strings_to_board_dict(self, board_strings):
-        return_dict = {}
-        for index, board in enumerate(board_strings):
+
+        def build_board(board):
             rows = re.findall(r"\d{9}", board)
             board_list = []
             for row in rows:
                 row_list = []
                 row_list[:0] = row
                 board_list.append(row_list)
-            return_dict[str(index)] = board_list
+            return board_list
+
+        return_dict = {}
+        for index, board in enumerate(board_strings):
+            return_dict[str(index)] = build_board(board)
         return return_dict
 
     def print_board(self, board):

Run the tests. Commit to git. Again, this is making code as reviewable as possible. Now move promote the function top level. We remove the self parameter, as it is not used.

diff --git a/treesudoku/tree_sudoku.py b/treesudoku/tree_sudoku.py
index 6a8cc15..316df52 100644
--- a/treesudoku/tree_sudoku.py
+++ b/treesudoku/tree_sudoku.py
@@ -87,15 +87,6 @@ class SudokuSolver:
 
     def strings_to_board_dict(self, board_strings):
 
-        def build_board(board):
-            rows = re.findall(r"\d{9}", board)
-            board_list = []
-            for row in rows:
-                row_list = []
-                row_list[:0] = row
-                board_list.append(row_list)
-            return board_list
-
         return_dict = {}
         for index, board in enumerate(board_strings):
             return_dict[str(index)] = build_board(board)
@@ -114,6 +105,16 @@ class SudokuSolver:
                 print('-' * 21)
 
 
+def build_board(board):
+    rows = re.findall(r"\d{9}", board)
+    board_list = []
+    for row in rows:
+        row_list = []
+        row_list[:0] = row
+        board_list.append(row_list)
+    return board_list
+
+

With this code promoted, we can now opt to promote the print_board method to top level function as well. Note that this is an untested function, and you should run the code from the command line to visually inspect it for this change.

diff --git a/treesudoku/tree_sudoku.py b/treesudoku/tree_sudoku.py
index 316df52..9dc5fe6 100644
--- a/treesudoku/tree_sudoku.py
+++ b/treesudoku/tree_sudoku.py
@@ -92,17 +92,18 @@ class SudokuSolver:
             return_dict[str(index)] = build_board(board)
         return return_dict
 
-    def print_board(self, board):
-        for index1, row in enumerate(board):
-            if index1 == 0 or index1 == 3 or index1 == 6:
-                print('-' * 21)
-            for index, char in enumerate(row):
-                print(char, '', end='')
-                if index == 2 or index == 5:
-                    print('| ', end='')
-            print('')
-            if index1 == 8:
-                print('-' * 21)
+
+def print_board(board):
+    for index1, row in enumerate(board):
+        if index1 == 0 or index1 == 3 or index1 == 6:
+            print('-' * 21)
+        for index, char in enumerate(row):
+            print(char, '', end='')
+            if index == 2 or index == 5:
+                print('| ', end='')
+        print('')
+        if index1 == 8:
+            print('-' * 21)
 
 
 def build_board(board):
@@ -217,7 +218,7 @@ start = time.time()
 solver = SudokuSolver(import_csv())
 for key, solution in solver.solved_board_strings.items():
     print(f"Board: {key}")
-    solver.print_board(solver.strings_to_board_dict([solution])['0'])
+    print_board(solver.strings_to_board_dict([solution])['0'])
 
 end = time.time()
 print(f"Finished in {end - start} seconds")

That is all the prep work. Next we have to tackle the algorithm at the heart of this code. While the bulk of that work is going to happen later, it so happens that the start of it is pulling out the tail end of the code into its own method, and that seems like an extension of what we have been doing here. Here’s the change:

diff --git a/treesudoku/tree_sudoku.py b/treesudoku/tree_sudoku.py
index 9dc5fe6..dc5a96b 100644
--- a/treesudoku/tree_sudoku.py
+++ b/treesudoku/tree_sudoku.py
@@ -76,7 +76,9 @@ class SudokuSolver:
                     curr_node.next()
                 else:
                     curr_node.next()
+        return self.build_solution_string(head_node)
 
+    def build_solution_string(self, head_node):
         return_string = ''
         curr_node = head_node
         return_string += str(curr_node.value)

It may happen that we want to move the logic for building this string into another object. If so, this is a pre-req. However, at a minimum, it documents the meaning of this section of code.

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.