4
\$\begingroup\$

This post is less of a question and more of a hope for people to see my code which I know is far from great and give me some direction or simple tips on improving the quality of it.

Normal Stud:

Normal Stud

Active Stud:

Active Stud

The program is a small simple class which inherits tkinter's Canvas object and uses it to create a 'for purpose' Touchscreen Pattern Password. It uses Images which have been attached and need to be placed in the same folder as the code which are for the pattern studs and then uses a series of bind events to know where the touch presses happen.

Have a play around if you wish and let me know any structure / coding improvement I could make. FYI this program was coded with python3.6 on windows operating system :)

Code:

import tkinter as tk

class PatternPassword(tk.Canvas):

    def __init__(self, show_pattern=False, show_numbers=False, max_length=9):

        super().__init__()

        if 1 < max_length > 9:
            #print('[*] Not aloud more than 9 as max_length')
            raise Exception('[*] Max length must be between 1 and 9')

        self.config(bg='grey', width=300, height=300)
        self.bind_all("<B1-Motion>", self.ShowInfo)
        self.bind_all("<ButtonPress-1>", self.ShowInfo)


        self.show_pattern = show_pattern
        self.show_numbers = show_numbers
        self.max_length = max_length

        self.pattern = tk.StringVar()
        self.pattern.set('Pattern Password: ')
        self.current_widget = None
        self.activeStub = tk.PhotoImage(file='stubActive.png')
        self.click_num = 0
        self.x1, self.y1, self.x2, self.y2 = None, None, None, None
        self.lines = []
        self.points = []

        self.SetupStubs()

    def AddLine(self, event):

        self.delete(self.lines[0])
        del self.lines[0]
        line = self.create_line(self.points, fill="white", arrow=tk.LAST, width=3)
        self.lines.append(line)

    def DrawLine(self, event, middleBounds):
       
        if self.click_num==0:
           self.x1=middleBounds[0]
           self.y1=middleBounds[1]
           self.click_num=1
           self.points.append(self.x1)
           self.points.append(self.y1)
        else:
           self.x2=middleBounds[0]
           self.y2=middleBounds[1]
           self.points.append(self.x2)
           self.points.append(self.y2)
           if len(self.lines) == 1:
               self.AddLine(event)
               return
           
           line = self.create_line(self.x1,self.y1,self.x2,self.y2, fill="white", width=3, arrow=tk.LAST, smooth=1, splinesteps=12)
           self.lines.append(line)

    def AddToPattern(self, number):

        self.pattern.set(f'Pattern Password: {self.pattern.get()[18:]}{str(number)}')

    def ActivateStub(self, number):

        self.itemconfig(self.stubs[number-1], image=self.activeStub)

    def ShowInfo(self, event):

        for stubNumber in list(self.stubs.values()):

            bound = self.bbox(stubNumber)
            x = [bound[0], bound[2]]
            y = [bound[1], bound[3]]
            middleBoundX = sum(x) / len(x)
            middleBoundY = sum(y) / len(y)
            middleBounds = [middleBoundX, middleBoundY]
            
            if bound[0] < event.x < bound[2] and bound[1] < event.y < bound[3]:

                widget = stubNumber     

                if self.current_widget != widget:
                    self.current_widget = widget
                    if len(self.pattern.get()) < (18+self.max_length) and str(self.current_widget) not in self.pattern.get()[18:]:
                        self.AddToPattern(self.current_widget)
                        self.ActivateStub(self.current_widget)
                        if self.show_pattern:
                            self.DrawLine(event, middleBounds)
                    
    def SetupStubs(self):

        x=20
        y=20

        self.stub = tk.PhotoImage(file='stub.png')
        self.stubs = {}

        for stubNum in range(9):

            stubButtonID = self.create_image(x,y,anchor=tk.NW,image=self.stub)

            x += 100

            if x == 320:
                y += 100
                x = 20

            self.stubs.update({stubNum: stubButtonID})
        
        if self.show_numbers:
            x=20
            y=20
            for stubNum in range(9):
                self.create_text(x+34, y+34, text=stubNum+1, fill="white", font=('Helvetica 15 bold'))
                x += 100

                if x == 320:
                    y += 100
                    x = 20

    def ClearPattern(self):

        self.pattern.set('Pattern Password: ')
        for stub in list(self.stubs.values()):
            #stub.config(image=self.stub)
            self.itemconfig(stub, image=self.stub)
        for line in self.lines:
            self.delete(line)
        self.click_num = 0
        self.points = []


if __name__ == '__main__':

    main = tk.Tk()
    main.geometry('500x500')
    main.config(bg='grey')

    title = tk.Label(main, text='Pattern Password', bg=main['bg'], fg='white', font=('Verdana Pro Light', 32, 'underline'))
    title.pack(fill=tk.X, pady=20)

    pattern = PatternPassword(show_pattern=True, show_numbers=False, max_length=9)
    pattern.pack()

    controlFrame = tk.Frame(main, bg='grey')
    controlFrame.pack_propagate(False)
    controlFrame.pack(padx=(50,0), pady=20, ipady=40, fill=tk.X, expand=1)

    passLabel = tk.Label(controlFrame, textvariable=pattern.pattern, font=('Verdana Pro Light', 18), bg='grey', fg='white')
    passLabel.pack(side=tk.LEFT)

    clearPattern = tk.Button(controlFrame, text='Clear', font=('Arial', 20), bg='grey', activebackground='grey', fg='white', activeforeground='white', bd=0, highlightthickness=0, command=pattern.ClearPattern)
    clearPattern.pack(side=tk.LEFT, padx=(20,0), ipadx=20, ipady=3)

    main.mainloop()


\$\endgroup\$
1
  • 2
    \$\begingroup\$ Please do not edit the question, especially the code, after an answer has been posted. Changing the question may cause answer invalidation. Everyone needs to be able to see what the reviewer was referring to. What to do after the question has been answered. \$\endgroup\$
    – pacmaninbw
    Commented Jun 24, 2022 at 13:53

2 Answers 2

3
\$\begingroup\$

Welcome to CodeReview! Your code is quite readable, but there are still ways to improve it:

Remove "magic" numbers

Whenever you write a number that is not 0 or 1 in your program think to yourself: what does this number represent?

self.config(bg='grey', width=300, height=300)

x += 100

if x == 320:
    y += 100
    x = 20

self.create_text(x+34, y+34, text=stubNum+1, fill="white", font=('Helvetica 15 bold'))

controlFrame.pack(padx=(50,0), pady=20, ipady=40, fill=tk.X, expand=1)

What are 300, 100, 320, 20, 34, 50 and so on? They are probably dimensions of your user interface but I am not sure exactly what they represent by glancing at the code. If I change just one of the 20s into 30 does the code still work, does the UI look weird?

At the start of your class you should do:

self.X_STEP = 20
self.WIDTH = 300
self.TEXT_OFFSET = 34

or better yet, pass these parameters to the __init__ method of your class to give the user a better costumization.

Function doing one thing only

def AddLine(self, event):

    self.delete(self.lines[0])
    del self.lines[0]
    line = self.create_line(self.points, fill="white", arrow=tk.LAST, width=3)
    self.lines.append(line)

Why does a function named AddLine also delete a line? At the very least you should add a comment saying why that needs to be deleted, but better yet, the function should do only what it says in the name.

Remove repetition

    x=20
    y=20

    self.stub = tk.PhotoImage(file='stub.png')
    self.stubs = {}

    for stubNum in range(9):

        stubButtonID = self.create_image(x,y,anchor=tk.NW,image=self.stub)

        x += 100

        if x == 320:
            y += 100
            x = 20

        self.stubs.update({stubNum: stubButtonID})
    
    if self.show_numbers:
        x=20
        y=20
        for stubNum in range(9):
            self.create_text(x+34, y+34, text=stubNum+1, fill="white", font=('Helvetica 15 bold'))
            x += 100

            if x == 320:
                y += 100
                x = 20

The code calculating the x,y position is duplicated here, the positions should be calculated once at the start and saved in a list to be reused.

Simplification

I am not 100% sure, but in the DrawLine function:

def DrawLine(self, event, middleBounds):

You should be able to just draw the line without saving the points in a list to simplify the code.

\$\endgroup\$
2
  • 1
    \$\begingroup\$ Hi! Thankyou very much for these comments. I can see the points you are making and will edit the code accordingly once done. \$\endgroup\$
    – Jack
    Commented Jun 24, 2022 at 8:38
  • 1
    \$\begingroup\$ @Jack You are welcome, please remember not to modify the code in the question, otherwise future viewers will not understand the context of the question. You are free to ask a new follow-up question after improving the code to ask for further improvements. \$\endgroup\$
    – Caridorc
    Commented Jun 24, 2022 at 17:39
-1
\$\begingroup\$

Here is the revised Code...

import tkinter as tk

class PatternPassword(tk.Canvas):
    '''
    Pattern Password Class which creates a 3x3 grid of studs where
    the user is able to input a code relative to a pattern
    corresponding to a code

    Three arguments are available:
      [+] show_pattern
        # Will Show the user what order they are selecting the studs
        [-] True
        [-] False 
      [+] show_numbers
        # Will Show numbers corresponding to each studs code value
        [-] True
        [-] False
      [+] max_length
        # Will determine the length of the pattern the user is allowed to make
        [-] 1-9 (Integer)
    '''
def __init__(self, show_pattern=False, show_numbers=False, max_length=9):

    super().__init__()

    #CONSTANTS
    LOWEST_LENGTH = 1
    HIGHEST_LENGTH = 9

    CANVAS_WIDTH = 300
    CANVAS_HEIGHT = 300

    self.STUD_START_X, self.STUD_START_Y = 20, 20
    self.STUD_ADD_X, self.STUD_ADD_Y = 100, 100
    self.STUD_NEXTROWVAL_X = 320
    self.X, self.Y = self.STUD_START_X, self.STUD_START_Y
    self.TEXT_DISPLACEMENT = 34

    if LOWEST_LENGTH < max_length > HIGHEST_LENGTH:
        raise Exception('[*] Max length must be between 1 and 9')

    #root canvas configuration and bindings
    self.config(bg='grey', width=CANVAS_WIDTH, height=CANVAS_HEIGHT)
    self.bind_all("<B1-Motion>", self.ShowInfo)
    self.bind_all("<ButtonPress-1>", self.ShowInfo)

    #make arguments available to all class methods
    self.show_pattern = show_pattern
    self.show_numbers = show_numbers
    self.max_length = max_length

    #class pattern variable
    self.pattern = tk.StringVar()
    self.pattern.set('Pattern Password: ')

    #cache variables
    self.current_widget = None
    self.line = None
    self.x1, self.y1, self.x2, self.y2 = None, None, None, None

    self.activeStub = tk.PhotoImage(file='stubActive.png')
    self.stub = tk.PhotoImage(file='stub.png')

    self.hasClicked = False
    
    self.points = []
    self.stubs = {}

    #Setup the widgets in the canvas
    self.SetupStubs()

def DeleteLine(self):

    #Deletes the current line on the canvas
    self.delete(self.line)

def AddLine(self):

    #Creates a new line on the canvas and assigns to class variable
    self.line = self.create_line(self.points, fill="white", arrow=tk.LAST, width=3)

def DrawLine(self, middleBounds):

    #Creates a new line point and then will try to call the AddLine method

    self.x = middleBounds[0]
    self.y = middleBounds[1]

    self.points.append(self.x)
    self.points.append(self.y)

    if self.hasClicked:
        self.AddLine()
    else:
        self.hasClicked = True

def AddToPattern(self, number):

    #Adds stud code number to the total password
    self.pattern.set(f'Pattern Password: {self.pattern.get()[18:]}{str(number)}')

def ActivateStub(self, number):

    #Makes stud turn green when pressed
    self.itemconfig(self.stubs[number-1], image=self.activeStub)

def ShowInfo(self, event):

    '''
    Will be called everytime the mouse button 1 is pressed down
    and moved. This method will constantly get the coordinates of
    the mouse and compare them with the bounds of each stud.
    If the mouse is inside one of the studs bounds and is not
    the 'self.current_widget' selected then it will be made
    the current widget and methods will be called on it
    to highlight it has been selected and to draw the lines etc.
    '''

    for stubNumber in list(self.stubs.values()):

        bound = self.bbox(stubNumber)
        
        if bound[0] < event.x < bound[2] and bound[1] < event.y < bound[3]:

            widget = stubNumber 

            x = [bound[0], bound[2]]
            y = [bound[1], bound[3]]
            middleBoundX = sum(x) / len(x)
            middleBoundY = sum(y) / len(y)
            middleBounds = [middleBoundX, middleBoundY] 

            if self.current_widget != widget:
                self.current_widget = widget
                if len(self.pattern.get()) < (18+self.max_length) and str(self.current_widget) not in self.pattern.get()[18:]:
                    self.AddToPattern(self.current_widget)
                    self.ActivateStub(self.current_widget)
                    if self.show_pattern:
                        self.DeleteLine()
                        self.DrawLine(middleBounds)
                
def SetupStubs(self, text=False):

    self.X, self.Y = self.STUD_START_X, self.STUD_START_Y

    for stubNum in range(9):

        if text:
            textID = self.create_text(self.X+self.TEXT_DISPLACEMENT, self.Y+self.TEXT_DISPLACEMENT, text=stubNum+1, fill="white", font=('Helvetica 15 bold'))
        else:
            stubButtonID = self.create_image(self.X,self.Y,anchor=tk.NW,image=self.stub)
            self.stubs.update({stubNum: stubButtonID})
        
        self.X += self.STUD_ADD_X
        if self.X == self.STUD_NEXTROWVAL_X:
            self.Y += self.STUD_ADD_Y
            self.X = self.STUD_START_X

    if not text:
        if self.show_numbers:
            self.SetupStubs(True)

def ClearPattern(self):

    for stub in list(self.stubs.values()):
        self.itemconfig(stub, image=self.stub)

    self.pattern.set('Pattern Password: ')
    self.delete(self.line)
    self.hasClicked = False
    self.points = []


if __name__ == '__main__':

main = tk.Tk()
main.geometry('500x500')
main.config(bg='grey')

title = tk.Label(main, text='Pattern Password', bg=main['bg'], fg='white', font=('Verdana Pro Light', 32, 'underline'))
title.pack(fill=tk.X, pady=20)

pattern = PatternPassword(show_pattern=True, show_numbers=False, max_length=9)
pattern.pack()

controlFrame = tk.Frame(main, bg='grey')
controlFrame.pack_propagate(False)
controlFrame.pack(padx=(50,0), pady=20, ipady=40, fill=tk.X, expand=1)

passLabel = tk.Label(controlFrame, textvariable=pattern.pattern, font=('Verdana Pro Light', 18), bg='grey', fg='white')
passLabel.pack(side=tk.LEFT)

clearPattern = tk.Button(controlFrame, text='Clear', font=('Arial', 20), bg='grey', activebackground='grey', fg='white', activeforeground='white', bd=0, highlightthickness=0, command=pattern.ClearPattern)
clearPattern.pack(side=tk.LEFT, padx=(20,0), ipadx=20, ipady=3)

main.mainloop()
\$\endgroup\$
1

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.