I have made a simple Sudoku solver in python and TKinter. However, I have used code from many different sources, so it is not neat. I can only write ugly code, so it is just about impossible for me to neaten it fully. I have made a start, but it is still bad. Here's what I have so far:
## IMPORTS ##
import tkinter as tk
from tkinter import ttk
from tkinter.messagebox import showinfo
## VARIABLES ##
count = 0
## METHODS ##
def board_to_list(board):
entryboard = [[],[],[],[],[],[],[],[],[]]
for row in range(9):
for item in range(9):
try:
if (board[row][item].get() == ""):
entryboard[row].append(-1)
elif not(int(board[row][item].get()) in [1,2,3,4,5,6,7,8,9]):
raise ValueError
else:
entryboard[row].append(int(board[row][item].get()))
except:
showinfo(message="Invalid sudoku")
return False
return entryboard
def find_next_empty(puzzle):
for row in range(9):
for column in range(9):
if puzzle[row][column] == -1:
return row, column
return None, None
def is_valid(puzzle, guess, row, col):
row_vals = puzzle[row]
if guess in row_vals:
return False
col_vals = [puzzle[i][col] for i in range(9)]
if guess in col_vals:
return False
row_start = (row // 3) * 3
col_start = (col // 3) * 3
for r in range(row_start, row_start + 3):
for c in range(col_start, col_start + 3):
if puzzle[r][c] == guess:
return False
return True
def solve_sudoku(puzzle):
global count
row, col = find_next_empty(puzzle)
if row is None and col is None:
return True
for guess in range(1,10):
count += 1
if is_valid(puzzle, guess, row, col):
puzzle[row][col] = guess
if solve_sudoku(puzzle):
return True
puzzle[row][col] = -1
return False
def is_impossible(puzzle):
for i in range(9):
row = {}
column = {}
block = {}
row_cube = 3 * (i//3)
column_cube = 3 * (i%3)
for j in range(9):
if puzzle[i][j]!= -1 and puzzle[i][j] in row:
return False
row[puzzle[i][j]] = 1
if puzzle[j][i]!=-1 and puzzle[j][i] in column:
return False
column[puzzle[j][i]] = 1
rc= row_cube+j//3
cc = column_cube + j%3
if puzzle[rc][cc] in block and puzzle[rc][cc]!=-1:
return False
block[puzzle[rc][cc]]=1
return True
def handle_solve_click(event):
global count
count = 0
entryboard = board_to_list(board)
if not entryboard:
return False
if not(is_impossible(entryboard)):
showinfo(message="Invalid sudoku")
return False
solve_sudoku(entryboard)
time = count/5
while time > 10000:
time -= 1000
print(time)
pb.start(round(time/100))
window.after(round(time), show_solution, entryboard)
window.after(10, update_progress_bar)
def show_solution(entryboard):
count = 0
for row in range(9):
for item in range(9):
board[row][item].delete(0, tk.END)
board[row][item].insert(0, entryboard[row][item])
print("+" + "---+"*9)
for i, row in enumerate(entryboard):
print(("|" + " {} {} {} |"*3).format(*[x if x != -1 else " " for x in row]))
if i % 3 == 2:
print("+" + "---+"*9)
else:
print("+" + " +"*9)
pb.stop()
pb['value'] = 100
def handle_clear_click(event):
for row in range(9):
for item in range(9):
board[row][item].delete(0, tk.END)
pb['value'] = 0
progress['text'] = "0.0%"
def handle_hint_click(event):
entryboard = board_to_list(board)
otherboard = board_to_list(board)
if not(entryboard):
return False
if not(is_impossible(entryboard)):
showinfo(message="Impossible")
return False
solve_sudoku(entryboard)
for row in range(9):
for item in range(9):
if otherboard[row][item] != entryboard[row][item]:
board[row][item].delete(0, tk.END)
board[row][item].insert(0, entryboard[row][item])
return True
showinfo(message="Already solved")
def update_progress_bar():
if pb['value'] < 100:
progress['text'] = f"{pb['value']}%"
window.after(10, update_progress_bar)
else:
pb['value'] = 100
progress['text'] = "Complete"
## MAIN LOOP ##
if __name__ == "__main__":
window = tk.Tk()
board = [[],[],[],[],[],[],[],[],[]]
entryboard = [[-1,-1,-1,-1,-1,-1,-1,-1,-1,],[-1,-1,-1,-1,-1,-1,-1,-1,-1,],[-1,-1,-1,-1,-1,-1,-1,-1,-1,],[-1,-1,-1,-1,-1,-1,-1,-1,-1,],[-1,-1,-1,-1,-1,-1,-1,-1,-1,],[-1,-1,-1,-1,-1,-1,-1,-1,-1,],[-1,-1,-1,-1,-1,-1,-1,-1,-1,],[-1,-1,-1,-1,-1,-1,-1,-1,-1,],[-1,-1,-1,-1,-1,-1,-1,-1,-1,]]
sudoku_frame = tk.Frame(relief=tk.SUNKEN, borderwidth=5)
for row in range(9):
for item in range(9):
myentry = tk.Entry(master=sudoku_frame, width=1)
row_start = (row // 3)
col_start = (item // 3)
rowpos = row_start + row
colpos = col_start + item + 1
myentry.grid(row=rowpos, column=colpos)
board[row].append(myentry)
sudoku_frame.pack()
progress_frame = tk.Frame()
pb = ttk.Progressbar(master=progress_frame, orient='horizontal', mode='determinate', length=280)
pb.grid(row=0, column=0)
progress = tk.Label(master=progress_frame, text="0.0%")
progress.grid(row=1, column=0)
progress_frame.pack(pady=15)
button_frame = tk.Frame(relief=tk.RIDGE, borderwidth=5)
solve_btn = tk.Button(master=button_frame, text="Solve", relief=tk.FLAT, borderwidth=2)
solve_btn.bind("<Button-1>", handle_solve_click)
solve_btn.grid(row=0,column=0)
clear_btn = tk.Button(master=button_frame, text="Clear", relief=tk.FLAT, borderwidth=2)
clear_btn.bind("<Button-1>", handle_clear_click)
clear_btn.grid(row=0, column=1)
hint_btn = tk.Button(master=button_frame, text="Hint", relief=tk.FLAT, borderwidth=2)
hint_btn.bind("<Button-1>", handle_hint_click)
hint_btn.grid(row=0, column=3)
button_frame.pack()
window.mainloop()
1 Answer 1
First, your program works. So that's a great place to start.
Some cosmetic comments:
- Put blank lines between your functions. This helps people reading your code to know when something new is happening.
- Put spaces between operators like
+,-,==,!=, etc. to make the lines easier to read. - The sudoku boxes in the GUI window are rather narrow and difficult to click on and enter numbers. Making the boxes wider and the text entry centered looks better:
myentry = tk.Entry(master=sudoku_frame, width=3, justify='center')
Initializing repetitive structures
In several places, you initialize sudoku data with literal lists and lists of lists like these:
board = [[],[],[],[],[],[],[],[],[]]
entryboard = [[-1,-1,-1,-1,-1,-1,-1,-1,-1,],[-1,-1,-1,-1,-1,-1,-1,-1,-1,],[-1,-1,-1,-1,-1,-1,-1,-1,-1,],[-1,-1,-1,-1,-1,-1,-1,-1,-1,],[-1,-1,-1,-1,-1,-1,-1,-1,-1,],[-1,-1,-1,-1,-1,-1,-1,-1,-1,],[-1,-1,-1,-1,-1,-1,-1,-1,-1,],[-1,-1,-1,-1,-1,-1,-1,-1,-1,],[-1,-1,-1,-1,-1,-1,-1,-1,-1,]]
You can use list comprehension to create these structures. The resulting code is shorter and easier to read.
board = [[] for _ in range(9)]
entryboard = [[-1 for _ in range(9)] for _ in range(9)]
The variable _ is often used as a dummy variable where we don't care about its value. We just need it to store the current value out of range(9).
Similar changes can be made to entryboard in board_to_list(). In that same function, elif not(int(board[row][item].get()) in [1,2,3,4,5,6,7,8,9]): can be written elif not(int(board[row][item].get()) in range(1, 10):
Initializing the entry grid
I don't understand the math being done in the loop creating all the tk.Entry widgets. This seems to work just as well.
for row in range(9):
for item in range(9):
myentry = tk.Entry(master=sudoku_frame, width=3, justify='center')
myentry.grid(row=row, column=item)
board[row].append(myentry)
Global variables
I'm not talking about count at the top. That variable is modified in several functions and is declared global in those functions. That's fine and suits its purpose.
I'm mostly talking about the board variable that is created on the second line of the if __name__ == "__main__": block. This variable holds all of the text entry widgets that are used to both allow the user to enter numbers and display numbers when the GUI solves a cell. In almost all functions, the board variable is the one created at the bottom of the code. There's no indication where the board variable is defined when looking at the function. I understand that the bind() method that binds the functions to button clicks only take an Event as its only argument, but there's a way to sneak the board and any other variables you need into the callback.
As an example, let's look at the handle_clear_click() function. First, change the parameters of the called function to take a board argument.
def handle_clear_click(event, board, pb, progress):
for row in range(9):
for item in range(9):
board[row][item].delete(0, tk.END)
pb['value'] = 0
progress['text'] = "0.0%"
Then, instead of directly passing just the function to the bind() call, create a lambda expression that provides the parameters that the bind() call cannot.
clear_btn.bind("<Button-1>", lambda event: handle_clear_click(event, board, pb, progress))
Now, there's no ambiguity in where the variables board, pb, and progress come from. Plus, this allows for easier changes later on. What if you want to allow work on more than one sudoku board in the same GUI? While that may not be likely for this program, other programs you write will have to juggle multiple sets of data.
Do the same thing for the other two buttons: Solve and Hint.
Unused data
The entryboard variable in the if __name__ == "__main__": block is not used anywhere, so you can delete it. The line count = 0 in show_solution() either needs to be deleted or have global put before it to reset the global count variable.
Several functions' return values that are not used. For example, handle_solve_click() returns False when the puzzle cannot be solved and nothing (implicitly None) otherwise. These return values don't do anything useful, so a simple return statement with nothing after it is fine.
def handle_solve_click(event):
global count
count = 0
entryboard = board_to_list(board)
if not entryboard:
return
if is_impossible(entryboard):
showinfo(message="Invalid sudoku")
return
solve_sudoku(entryboard)
time = count/5
while time > 10000:
time -= 1000
print(time)
pb.start(round(time/100))
window.after(round(time), show_solution, entryboard)
window.after(10, update_progress_bar)
Semantic bug
The function is_impossible() returns False when the puzzle is impossible to solve. This is the reverse of what I would expect given the name. I would switch True and False in all the return values of this function and fix the handle_hint_click() and handle_solve_click() functions so that the lines if not(is_impossible(entryboard)): do not have a not. The other choice is to rename the function is_possible().