M D
M D

Reputation: 51

Unable to modify variable outside of the method

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

Answers (2)

ggorlen
ggorlen

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

gelonida
gelonida

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

Related Questions