-2

I have been working on a python program to solve a sudoku puzzle, but I am not sure what I did wrong. I thought i had fixed it but now I cannot get it to print the board or solve it. I have already written the algorithm to solve as well as the function to create the board. If someone can show me where it went wrong id appreciate it. Heres my code (Sorry about indentation errors-they were caused by copy and pasting):

    class Sudoku:
def __init__(self, board, cells):
    self.board = board
    self.cells = cells

#Creates a board
def newboard(self):
    values = self.cells.split(" ")
    val_counter = 0

    if len(values) != 81:
        print("Error: Not enough values.")
        exit()

    else:
        for i in range(9):
            for j in range(9):
                self.board[i][j] = values[val_counter]
                val_counter += 1

# Returns row
def findValidRow(self):
    for i in range(9):
        for j in range(9):
            if int(self.board[i][j]) == 0:
                return int(i)
    return -1

# Returns col
def findValidCol(self):
    for i in range(9):
        for j in range(9):
            if int(self.board[i][j]) == 0:
                return int(j)
    return -1

def possible(self, row, col, val):
    # Check row for value
    for i in range(9):
        if self.board[row][i] == val:
            return False
    # Checks col for value
    for j in range(9):
        if self.board[j][col]:
            return False
    # Checks square for value
    coordX, coordY = 3 * (row // 3), 3 * (col // 3)
    for x in range(coordX, coordX + 3):
        for y in range(coordY, coordY + 3):
            if coordX == row and coordY == col:
                continue
            if self.board[coordX][coordY] == val:
                return False
    return True

    # Solves the board

def solve(self):
    # Checks if cells are all solved
    if self.findValidCol() == -1:
        print(self.board)
        return True

    # Finds first cell to fill
    row = self.findValidRow()
    col = self.findValidCol()

    for i in range(1, 10):
        if self.possible(row, col, i):
            self.board[row][col] = i
            # Updates values to find new cell to fill

            if self.solve():
                return True
            # Backtracks
            self.board[row][col] = 0

    return False


    # Get cell values and calls solve function
    get_cells = input("Enter cell values seperated by 1 space. Enter 0 for empty cells: ")
    b = Sudoku([[0, 0, 0, 0, 0, 0, 0, 0, 0], [0, 0, 0, 0, 0, 0, 0, 0, 0], [0, 0, 0, 0, 0, 0, 0,    0, 0], [0, 0, 0, 0, 0, 0, 0, 0, 0],[0, 0, 0, 0, 0, 0, 0, 0, 0], [0, 0, 0, 0, 0, 0, 0, 0, 0], [0, 0, 0, 0, 0, 0, 0, 0, 0], [0, 0, 0, 0, 0, 0, 0, 0, 0],[0, 0, 0, 0, 0, 0, 0, 0, 0]], get_cells)
    b.newboard()
    b.solve()
cdlane
  • 40,441
  • 5
  • 32
  • 81
qmayo14
  • 11
  • 3
  • You have two separate functions for finding valid row,col. Are you aware you can return both values with a single function?, e.g. `row,col = self.findValidCell()` and within the `findValidCell` function, change the return to `return row,col` – RufusVS Aug 20 '20 at 02:01
  • You only have one print statement, is that what is not executing? – RufusVS Aug 20 '20 at 02:02
  • I tested a bit with your solve and it seems to not be working; it's not printing anything because it's outputting false when I'm using a valid board – jreiss1923 Aug 20 '20 at 02:14
  • @RufusVS As far as I know, yes. The debugger in VScode doesn’t go through the functions when they are called at the end of the code. I believe it is the print function that the condition to call it is never met. If it is printing then maybe I messed up something in the newboard() function. If you were able to determine the source of the issue then maybe I’m wrong, but I can’t find it in VScode. – qmayo14 Aug 20 '20 at 02:20

2 Answers2

0

Your first problem is in here:

# Check row for value
for i in range(9):
    if self.board[row][i] == val:
        return False
# Checks col for value
for j in range(9):
    if self.board[j][col]:
        return False

Specifically, the line:

if self.board[j][col]:

which should be:

if self.board[j][col] == val:

The next problem is in here:

coordX, coordY = 3 * (row // 3), 3 * (col // 3)
for x in range(coordX, coordX + 3):
    for y in range(coordY, coordY + 3):
        if coordX == row and coordY == col:
            continue
        if self.board[coordX][coordY] == val:
            return False

Where you're using coordX and coordY in the loop when I think you should be using x and y:

for x in range(coordX, coordX + 3):
    for y in range(coordY, coordY + 3):
        if x == row and y == col:
            continue
        if self.board[x][y] == val:
            return False

Otherwise, you're repeating the same test over and over. A final problem I saw when printing the board, is that your board's contents were a mix of int and str -- you need to get a handle on this early and only use one of them.

Below is my rework of your code which solves a test Sudoku I had on hand:

class Sudoku:
    def __init__(self, board, cells):
        self.board = board
        self.cells = cells

    def newBoard(self):
        ''' Creates a board '''

        values = self.cells.split(" ")

        assert len(values) == 81, "Error: Not enough values."

        for i in range(9):
            for j in range(9):
                self.board[i][j] = int(values.pop(0))

    def findValidCell(self):
        ''' Returns empty cell indicies or None '''

        for row in range(9):
            for col in range(9):
                if self.board[row][col] == 0:
                    return (row, col)
        return None

    def possible(self, row, col, val):
        # Check row for value
        for i in range(9):
            if self.board[row][i] == val:
                return False

        # Checks col for value
        for j in range(9):
            if self.board[j][col] == val:
                return False

        # Checks square for value
        coordX, coordY = 3 * (row // 3), 3 * (col // 3)

        for x in range(coordX, coordX + 3):
            for y in range(coordY, coordY + 3):
                if x == row and y == col:
                    continue

                if self.board[x][y] == val:
                    return False

        return True

    def printBoard(self):
        print(*self.board, sep='\n')

    def solve(self):
        ''' Solves the board '''

        # Checks if cells are all solved
        cell = self.findValidCell()

        if cell is None:
            return True

        # Finds first cell to fill
        row, col = cell

        for i in range(1, 10):
            if self.possible(row, col, i):
                self.board[row][col] = i
                # Updates values to find new cell to fill

                if self.solve():
                    return True

                # Backtracks
                self.board[row][col] = 0

        return False

# Get cell values and calls solve function
get_cells = input("Enter cell values separated by 1 space. Enter 0 for empty cells: ")
b = Sudoku([
    [0, 0, 0, 0, 0, 0, 0, 0, 0],
    [0, 0, 0, 0, 0, 0, 0, 0, 0],
    [0, 0, 0, 0, 0, 0, 0, 0, 0],
    [0, 0, 0, 0, 0, 0, 0, 0, 0],
    [0, 0, 0, 0, 0, 0, 0, 0, 0],
    [0, 0, 0, 0, 0, 0, 0, 0, 0],
    [0, 0, 0, 0, 0, 0, 0, 0, 0],
    [0, 0, 0, 0, 0, 0, 0, 0, 0],
    [0, 0, 0, 0, 0, 0, 0, 0, 0],
    ], get_cells)

b.newBoard()
if b.solve():
    b.printBoard()
cdlane
  • 40,441
  • 5
  • 32
  • 81
0

The answer @cdlane gave worked great. I just noticed a couple optimizations I would use to get marginal performance improvement in the possible function, resulting in:

def possible(self, row, col, val):
    # Check row or column for value
    for i in range(9):
        if (self.board[row][i] == val) or (self.board[i][col] == val):
            return False

    # Checks 3x3 containing square for value
    coordX, coordY = 3 * (row // 3), 3 * (col // 3)

    for x in range(coordX, coordX + 3):
        for y in range(coordY, coordY + 3):
            if self.board[x][y] == val:
                return False
    return True

You can see I check row and column within the same for loop, and I eliminated an unnecessary cell location check because the candidate cell contains zero at that point.

And because I like more information in my error messages, I modified the assert to be:

    assert len(values) == 81, "Error: {} values {} != {}".format(("Not enough"if len(values)<81 else "Too many"),len(values),81)
RufusVS
  • 4,008
  • 3
  • 29
  • 40