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()
Sᴀᴍ Onᴇᴌᴀ
29.5k16 gold badges45 silver badges201 bronze badges
asked Jun 23, 2022 at 13:28
\$\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\$ 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.

answered Jun 23, 2022 at 17:54
\$\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\$ 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\$ 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()
answered Jun 24, 2022 at 14:16
\$\endgroup\$
1

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.