2
\$\begingroup\$

I made OOP sudoku generator - at this moment it generates only valid table. You can clear it and then generate over.

The case is I struggled with calling "fill_board" and "clear_board" form the button calss. So I made that class method, otherwise instance error appeard. It is finally working as intended but can you comment the code? I think it can be defined way easier.

from tkinter import * from random import randint

class Buttons:
    """
    Initiate buttons and all of features
    """
    
    def __init__(self, window, label, x, y, width=5, height=2, rowspan=1):
        """
        Key variables,
        Depending on the position of the button assing the command
        """
        self.button_font = ('Verdana', 15)
        self.parent = window
        self.__pos_x = x
        self.__pos_y = y
        self.rowspan = rowspan
        self.skip = []
        
        self.height = height
        self.width = width

        self.container = Frame(self.parent)
        self.container.grid(row=self.__pos_x, column=self.__pos_y)


        if self.__pos_x <= 9 and self.__pos_y <= 9:
            self.com = lambda: self.click()
            self.num = label
            self.text = self.num            
        else:
            self.text = label      
            if self.text == Board._label[0]:
                self.com = lambda: Board.fill_board(self)
            else:
                self.com = lambda: Board.new_board()
        self.button()    

    def button(self):
        """
        button method
        """
        self.b = Button(
            self.parent, text=self.text, width=self.width,
            height=self.height, font=self.button_font,
            command=self.com )
        self.b.grid(row=self.__pos_x, column=self.__pos_y, rowspan = self.rowspan)
        
        
    def click(self):
        """
        Creates a window to input number for sudoku
        """
        self.top=Toplevel(self.parent)
        self.l=Label(self.top,text="Sudoku value")
        self.l.pack()
        self.e=Entry(self.top)
        self.e.pack()
        self.bi=Button(self.top,text='Ok',command=self.cleanup)
        self.bi.pack()
        
    def cleanup(self):
        self.num = int(self.e.get())

        self.b["text"] = self.num
        self.top.destroy()


class Board():
    """
    Board class
    """
    
    size = 9
    container = Tk()
    
    _label = ["Generate sudoku", "Make empty board"]
    def __init__(self):
        """
        Generates empty board,
        Adds two additionall buttons
        Fills it out with buttons array

        """
        self.button_font = ('Verdana', 15)
        self.new_board()
        self.skip = []
        [Buttons(Board.container, Board._label[i], i*2, 12, 20, 5, 2) for i in range(len(Board._label))]
        self.fill_board()
    
    @classmethod
    def new_board(cls):
        """
        Class method to create empty board, 
        used to be instance method but could be called from button class as instance
        """
        cls.board = [[Buttons(cls.container, 0, i, j) for i in range(cls.size)] for j in range(cls.size)]

    
    def fill_board(self):
        """
        method that backtracing alghoritm to generate sudoku numbers, 
        stored as button label
        """
        if not Board.find_zero():
            return True
        for self.num_ in range(20):
            
            self.num_ = randint(0, 9)
            self.__pos_x, self.__pos_y = Board.find_zero()
            # print(self.skip)
            if Board.check_number(self):
                Board.board[self.__pos_x][self.__pos_y].b["text"] = self.num_

                self.skip = []
                if Board.fill_board(self):
                    return True
                self.skip = []
                if self.__pos_y == 0:
                    self.__pos_x -= 1
                    self.__pos_y = 8
                else:
                    self.__pos_y -= 1

                self.skip.append(Board.board[self.__pos_x][self.__pos_y].b["text"]) 
                Board.board[self.__pos_x][self.__pos_y].b["text"] = 0
        return False
    
       
    def check_number(self):
        """
        check if number appears in row, column or 3x3 square
        """
        

        self.col = []
        for i in range(len(Board.board)):
            self.col.append(Board.board[i][self.__pos_y].b["text"])


    
        self.lil_sqr = []
        for i in range(3):
            for j in range(3):
                self.lil_sqr.append(Board.board[(3*(self.__pos_x // 3)) + i][(3*(self.__pos_y // 3)) + j].b["text"])
        
        self.row = []
        for i in range(len(Board.board)):
            self.row.append(Board.board[self.__pos_x][i].b["text"])
        

        
        if self.num_ not in self.row \
            and self.num_ not in self.col \
                and self.num_ not in self.skip \
                    and self.num_ not in self.lil_sqr:
                
                return True
        self.skip.append(self.num_)
        return False
   
    @classmethod
    def find_zero(cls):
        """
        finds zero location
        """
        for i in range(9):
            for j in range(9):
                if cls.board[i][j].b["text"] == 0:
                    return (i, j)  # row, col



Board.container.title("grid method") 
Board.container.geometry("1000x800")         
sudoku = Board()  
Board.container.mainloop()
\$\endgroup\$
0

1 Answer 1

1
\$\begingroup\$

Avoid import *. For your case, it's easy enough to alias tkinter as import tkinter as tk, and then qualify all of your references with tk..

Your Buttons class is very troubled. Half of its functionality only applies to buttons within the Sudoku grid itself, and half applies to control buttons outside of the grid. These responsibilities should not be in the same class. Conditionally switching between them based on coordinates in the constructor is not a good idea. Moreover, conditionally switching between control button responsibilities based on presentation data (the label) is also not a good idea.

Lambdas are not called for here. Just write direct bindings to instance methods, i.e. my_board.new_board.

Add PEP484 type hints.

Don't prefix your member variables with double underscores; that holds a special name-mangling meaning that doesn't apply here.

self.button, first of all, should be called something like self.make_button (an action phrase) and not a noun. Also, it should not initialise new class members outside of the constructor; instead it should just return the new button instance and that should be set within the constructor.

Don't use single-letter variable names like l (label), e (entry), etc. Those two variables should not be set as member variables at all since no reference is needed to them.

Rather than Entry.get, consider the use of actual IntVar.

Don't suffix your class declarations with ().

Board._label probably shouldn't exist. There just isn't enough commonality between the two buttons to justify generalising their labels into a sequence. Just pass those labels as inline literals to the tk button constructor.

Board.container must not be a static. Make it an instance variable, or else your class will be non-reentrant.

additionall is spelled additional.

backtracing is spelled backtracking.

This expression:

[Buttons(Board.container, Board._label[i], i * 2, 12, 20, 5, 2) for i in range(len(Board._label))]

is useless as a list comprehension, because its value is immediately discarded. Re-express this as a normal for loop.

fill_board is very slow. I encourage you to scrutinise its algorithm; I have not done so.

Your code inappropriately intermixes the concern of presentation with the concern of logic. Your Sudoku board generation code should be entirely separated (even into a separate file) and have zero references to tk.

This constructor call:

Buttons(Board.container, Board._label[i], i * 2, 12, 20, 5, 2)

should use kwarg= syntax instead of positional syntax.

Your last four lines should be moved to a main() method which is in turn called by an if __name__ == '__main__': guard.

Redundant comments like """Board class""" need to go away.

Don't for i in range(len(Board._label)); use enumerate instead.

Suggested

Addressing some, but certainly not all, of the above suggestions:

import tkinter as tk
from random import randint
from typing import Optional, Union


class Buttons:
    def __init__(
        self,
        window: tk.Tk,
        board: 'Board',
        label: Union[int, str],
        x: int,
        y: int,
        width: int = 5,
        height: int = 2,
        row_span: int = 1,
    ) -> None:
        """
        Key variables,
        Depending on the position of the button passing the command
        """
        self.button_font = ('Verdana', 15)
        self.parent = window
        self.pos_x = x
        self.pos_y = y
        self.row_span = row_span
        self.skip = []

        self.height = height
        self.width = width

        self.container = tk.Frame(self.parent)
        self.container.grid(row=self.pos_x, column=self.pos_y)

        if self.pos_x <= 9 and self.pos_y <= 9:
            self.command = self.click
            self.num = label
            self.text = self.num
        else:
            self.text = label
            if self.text == Board.labels[0]:
                self.command = board.fill_board
            else:
                self.command = board.new_board
        self.make_button()

    def make_button(self) -> None:
        self.button = tk.Button(
            self.parent, text=self.text, width=self.width,
            height=self.height, font=self.button_font,
            command=self.command,
        )
        self.button.grid(row=self.pos_x, column=self.pos_y, rowspan=self.row_span)

    def click(self) -> None:
        """
        Creates a window to input number for sudoku
        """
        self.top = tk.Toplevel(self.parent)
        label = tk.Label(self.top, text="Sudoku value")
        label.pack()
        self.entry = tk.Entry(self.top)
        self.entry.pack()
        button = tk.Button(self.top, text='Ok', command=self.cleanup)
        button.pack()

    def cleanup(self) -> None:
        self.num = int(self.entry.get())

        self.button["text"] = self.num
        self.top.destroy()


class Board:
    labels = ["Generate sudoku", "Make empty board"]
    size = 9

    def __init__(
        self,
        title: str = "grid method",
        width: int = 1000,
        height: int = 800,
    ) -> None:
        """
        Generates empty board,
        Adds two additional buttons
        Fills it out with buttons array
        """
        self.container = tk.Tk()
        self.container.title(title)
        self.container.geometry(f'{width}x{height}')

        self.button_font = ('Verdana', 15)
        self.new_board()
        self.skip = []
        for i, label in enumerate(self.labels):
            Buttons(
                window=self.container, board=self, label=label,
                x=i * 2, y=12, width=20, height=5, row_span=2,
            )
        self.fill_board()

    def main_loop(self) -> None:
        self.container.mainloop()

    def new_board(self) -> None:
        """
        Class method to create empty board,
        used to be instance method but could be called from button class as instance
        """
        self.board = [
            [
                Buttons(window=self.container, board=self, label=0, x=i, y=j)
                for i in range(self.size)
            ] for j in range(self.size)
        ]

    def fill_board(self) -> bool:
        """
        method that backtracking algorithm to generate sudoku numbers,
        stored as button label
        """
        if not self.find_zero():
            return True

        for self.num_ in range(20):
            self.num_ = randint(0, 9)
            self.pos_x, self.pos_y = self.find_zero()

            if self.check_number():
                self.board[self.pos_x][self.pos_y].button["text"] = self.num_

                self.skip = []
                if self.fill_board():
                    return True
                self.skip = []
                if self.pos_y == 0:
                    self.pos_x -= 1
                    self.pos_y = 8
                else:
                    self.pos_y -= 1

                self.skip.append(self.board[self.pos_x][self.pos_y].button["text"])
                self.board[self.pos_x][self.pos_y].button["text"] = 0
        return False

    def check_number(self) -> bool:
        """
        check if number appears in row, column or 3x3 square
        """

        self.col = []
        for i in range(len(self.board)):
            self.col.append(self.board[i][self.pos_y].button["text"])

        self.lil_sqr = []
        for i in range(3):
            for j in range(3):
                self.lil_sqr.append(
                    self.board[
                        (3 * (self.pos_x // 3)) + i
                        ][
                        (3 * (self.pos_y // 3)) + j
                        ].button["text"])

        self.row = []
        for i in range(len(self.board)):
            self.row.append(self.board[self.pos_x][i].button["text"])

        if (
            self.num_ not in self.row
            and self.num_ not in self.col
            and self.num_ not in self.skip
            and self.num_ not in self.lil_sqr
        ):
            return True

        self.skip.append(self.num_)
        return False

    def find_zero(self) -> Optional[tuple[int, int]]:
        for i in range(9):
            for j in range(9):
                if self.board[i][j].button["text"] == 0:
                    return i, j  # row, col


def main() -> None:
    sudoku = Board()
    sudoku.main_loop()


if __name__ == '__main__':
    main()

I encourage you to work on the above, give another day or two to see if there are other suggestions from the community, and then post your modified code in a new question. Good luck!

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.