Reputation: 51
I'm trying to make a bot for a sudoku game. It seem to work, but there's a small problem the solved grid is only printed inside the method. When I run it, the print(game.board) print the unsolved one.
this is my code:
import numpy as np
class Game:
def solve(self):
def possible(y,x,n):
for i in range(9):
if self.board[y][i] == n:
return(False)
for i in range(9):
if self.board[i][x] == n:
return(False)
gridx = (x // 3) * 3
gridy = (y // 3) * 3
for i in range(3):
for j in range(3):
if self.board[gridy + i][gridx + j] == n:
return(False)
return(True)
def solving():
for y in range(9):
for x in range(9):
if self.board[y][x] == 0:
for n in range(1,10):
if possible(y,x,n):
self.board[y][x] = n
solving()
self.board[y][x] = 0
return
print(np.matrix(self.board))
solving()
game = Game()
game.board = [
[7,8,0,4,0,0,1,2,0],
[6,0,0,0,7,5,0,0,9],
[0,0,0,6,0,1,0,7,8],
[0,0,7,0,4,0,2,6,0],
[0,0,1,0,5,0,9,3,0],
[9,0,4,0,6,0,0,0,5],
[0,7,0,3,0,0,0,1,2],
[1,2,0,0,0,7,4,0,0],
[0,4,9,2,0,6,0,0,7]
]
game.solve()
print(game.board)
Upvotes: 2
Views: 80
Reputation: 57259
Currently, once a solution is found, the backtracking still sets each cell to zero and when the solve
call returns, you're left with your original board.
This design is quite awkward, though. Typically, you'd not reach into a class and set a self
property from outside:
game.board = [...]
This breaks encapsulation. The Game
class' logic will fail if the caller doesn't do this properly. Better to pass a parameter into the initializer or into the solve
function. Since solving sudoku is stateless and is done here with just a couple of functions, it feels odd to create an object.
Game
is a vague class name. SudokuSolver
is clearer.
Similarly, solving
is confusingly named; method names should be commands or actions, not participles.
return
is not a function: return True
rather than return(True)
.
Avoid 5-6 level nested loops and conditional blocks. Such code is difficult to reason about. Work can be saved by precomputing all of the empty positions once, then skipping all other squares.
Here's a quick rewrite with room for improvement:
class SudokuSolver:
@staticmethod
def possible_move(board, x, y, n):
for i in range(9):
if board[y][i] == n or board[i][x] == n:
return False
grid_x = x // 3 * 3
grid_y = y // 3 * 3
for i in range(3):
for j in range(3):
if board[grid_y+i][grid_x+j] == n:
return False
return True
@staticmethod
def solve(board):
empty_squares = [(x, y) for y, row in enumerate(board)
for x, n in enumerate(row) if n == 0]
def backtrack(i=0):
if i >= len(empty_squares):
return board
x, y = empty_squares[i]
for n in range(1, 10):
if SudokuSolver.possible_move(board, x, y, n):
board[y][x] = n
if soln := backtrack(i + 1):
return soln
board[y][x] = 0
return backtrack()
if __name__ == "__main__":
board = [
[7,8,0,4,0,0,1,2,0],
[6,0,0,0,7,5,0,0,9],
[0,0,0,6,0,1,0,7,8],
[0,0,7,0,4,0,2,6,0],
[0,0,1,0,5,0,9,3,0],
[9,0,4,0,6,0,0,0,5],
[0,7,0,3,0,0,0,1,2],
[1,2,0,0,0,7,4,0,0],
[0,4,9,2,0,6,0,0,7]
]
if soln := SudokuSolver.solve(board):
print("\n".join(",".join(map(str, x)) for x in soln))
else:
print("unsolvable board")
Upvotes: 2
Reputation: 5640
Your code is calling recursively solving() however, it unsets the tried out numbers afterwards.
What you had to do is abort as soon as you find a solution.
Just change solving, such, that it stops when a solution is found:
def solving():
for y in range(9):
for x in range(9):
if self.board[y][x] == 0:
for n in range(1,10):
if possible(y,x,n):
self.board[y][x] = n
solved = solving()
if solved:
return True
self.board[y][x] = 0
return False
return True
solving()
Bonus:
Here a flattened out version (without function nesting) of your solver:
I also added an __init__
function, so that the board is passed to the class.
import numpy as np
class Game:
def __init__(self, board):
self.board = board
def possible(self, y, x, n):
for i in range(9):
if self.board[y][i] == n:
return False
for i in range(9):
if self.board[i][x] == n:
return False
gridx = (x // 3) * 3
gridy = (y // 3) * 3
for i in range(3):
for j in range(3):
if self.board[gridy + i][gridx + j] == n:
return False
return True
def solving(self):
for y in range(9):
for x in range(9):
if self.board[y][x] == 0:
for n in range(1,10):
if self.possible(y,x,n):
self.board[y][x] = n
solved = self.solving()
if solved:
return True
self.board[y][x] = 0
return False
return True
def solve(self):
self.solving()
return self.board
board = [
[7,8,0,4,0,0,1,2,0],
[6,0,0,0,7,5,0,0,9],
[0,0,0,6,0,1,0,7,8],
[0,0,7,0,4,0,2,6,0],
[0,0,1,0,5,0,9,3,0],
[9,0,4,0,6,0,0,0,5],
[0,7,0,3,0,0,0,1,2],
[1,2,0,0,0,7,4,0,0],
[0,4,9,2,0,6,0,0,7]
]
game = Game(board)
print(np.matrix(game.board))
game.solve()
print(np.matrix(game.board))
Upvotes: 3