I have another thread called Digitizing paper label system with Python
I have added username and password class (class is called login()
).
I'd like to hear some feedback about my class login()
is the structure okay?
Should I change anything within my structure? Shall I make any other amendments, I will hear out any feedback.
You will see a lot of grid_forget
, because I wasn't sure how to structure my Classes, roots and how to close login class and load main app after successful log in.
The login app is near the bottom.
from tkinter import *
from tkinter import DISABLED, messagebox
import tkinter.ttk as ttk
import os
import glob
from PIL import Image, ImageTk, ImageGrab
from pathlib import Path
import pyautogui
# from openpyxl import load_workbook
class App():
def __init__(self,master):
notebook = ttk.Notebook(master)
notebook.pack(expand = 1, fill = "both")
#Frames
main = ttk.Frame(notebook)
manual = ttk.Frame(notebook)
notebook.add(main, text='Main-Screen')
notebook.add(manual, text='Manual')
#Check boxes
#Assigning Integers to variables
var1 = IntVar()
var2 = IntVar()
var3 = IntVar()
var4 = IntVar()
var5 = IntVar()
var6 = IntVar()
var7 = IntVar()
var8 = IntVar()
var9 = IntVar()
var10 = IntVar()
var11 = IntVar()
var12 = IntVar()
#Text boxes for initials
#Displaying checkboxes and assigning to variables
self.Checkbox1 = Checkbutton(main, text="Ingredients present in full (any allergens in bold with allergen warning if necessary)", variable=var1)
self.Checkbox1.grid(column = 2, row = 1, sticky = W)
self.Checkbox2 = Checkbutton(main, text="May Contain Statement.", variable=var2)
self.Checkbox2.grid(column = 2, row = 2, sticky = W)
self.Checkbox3 = Checkbutton(main, text="Cocoa Content (%).", variable=var3)
self.Checkbox3.grid(column = 2, row = 3, sticky = W)
self.Checkbox4 = Checkbutton(main, text="Vegetable fat in addition to Cocoa butter", variable=var4)
self.Checkbox4.grid(column = 2, row = 4, sticky = W)
self.Checkbox5 = Checkbutton(main, text="Instructions for Use.", variable=var5)
self.Checkbox5.grid(column = 2, row = 5, sticky = W)
self.Checkbox6 = Checkbutton(main, text="Additional warning statements (pitt/stone, hyperactivity etc)", variable=var6)
self.Checkbox6.grid(column = 2, row = 6, sticky = W)
self.Checkbox7 = Checkbutton(main, text="Nutritional Information Visible", variable=var7)
self.Checkbox7.grid(column = 2, row = 7, sticky = W)
self.Checkbox8 = Checkbutton(main, text="Storage Conditions", variable=var8)
self.Checkbox8.grid(column = 2, row = 8, sticky = W)
self.Checkbox9 = Checkbutton(main, text="Best Before & Batch Information", variable=var9)
self.Checkbox9.grid(column = 2, row = 9, sticky = W)
self.Checkbox10 = Checkbutton(main, text="Net Weight & Correct Font Size.", variable=var10)
self.Checkbox10.grid(column = 2, row = 10, sticky = W)
self.Checkbox11 = Checkbutton(main, text="Barcode - Inner", variable=var11)
self.Checkbox11.grid(column = 2, row = 11, sticky = W)
self.Checkbox12 = Checkbutton(main, text="Address & contact details correct", variable=var12)
self.Checkbox12.grid(column = 2, row = 12, sticky = W)
##DISABLE ON CLICK##
def showstate(*args):
if var1.get() or var2.get() or var3.get() or var4.get() or var5.get() or var6.get() or var7.get() or var8.get() or var9.get() or var10.get() or var11.get() or var12.get():
self.user = Label(main, text = "")
self.user.grid(column = 2, row = 0, sticky = N)
var1.trace_variable("w", showstate)
var2.trace_variable("w", showstate)
var3.trace_variable("w", showstate)
var4.trace_variable("w", showstate)
var5.trace_variable("w", showstate)
var6.trace_variable("w", showstate)
var7.trace_variable("w", showstate)
var8.trace_variable("w", showstate)
var9.trace_variable("w", showstate)
var10.trace_variable("w", showstate)
var11.trace_variable("w", showstate)
var12.trace_variable("w", showstate)
##DISABLE ON CLICK##
#Send data
def var_states():
text_file = open("logfile.txt", "a")
text_file.write()
text_file.close()
# pyautogui.keyDown('alt')
# pyautogui.keyDown('printscreen')
# pyautogui.keyUp('printscreen')
# pyautogui.keyUp('alt')
self.img = ImageGrab.grabclipboard()
self.img.save('paste.jpg', 'JPEG')
self.dataSend = Button(main, text = "Send", command = var_states).grid(column = 1, row = 13, sticky = W)
###################################################################################################################################
##Load Image##
###################################################################################################################################
# Create a Tkinter variable
self.tkvar = StringVar(root)
# Directory
self.directory = "//SERVER/shared_data/Technical/Label Sign Off Sheets/sign off project"
self.choices = glob.glob(os.path.join(self.directory, "*- to sign.jpg"))
self.tkvar.set('...To Sign Off...') # set the default option
# Images
def change_dropdown():
imgpath = self.tkvar.get()
img = Image.open(imgpath)
img = img.resize((529,361))
photo = ImageTk.PhotoImage(img)
label2.image = photo
label2.configure(image=photo)
#return path value
self.p = None
def func(value):
global p
self.p = Path(value)
print(self.p)
#widgets
self.msg1 = Label(main, text = "Choose here")
self.msg1.grid(column = 0, row = 0)
self.popupMenu = OptionMenu(main, self.tkvar, *self.choices, command = func)
self.popupMenu.grid(row=1, column=0)
self.display_label = label2 = Label(main, image=None)
self.display_label.grid(row=2, column=0, rowspan = 500)
self.open_button = Button(main, text="Open", command=change_dropdown)
self.open_button.grid(row=502, column=0)
###################################################################################################################################
##Tab 2 - Manual##
###################################################################################################################################
def open_doc():
os.system("start C:/Users/Eduards/Desktop/Portfolio")
self.Manual_Button = Button(manual, text = "Open Manual", command = open_doc)
self.Manual_Button.pack()
###################################################################################################################################
##USERS##
###################################################################################################################################
class login(Frame):
def __init__(self, master):
super().__init__(master)
self.label_username = Label(self, text="Username")
self.label_password = Label(self, text="Password")
self.entry_username = Entry(self)
self.entry_password = Entry(self, show="*")
self.label_username.grid(row=0, sticky=E)
self.label_password.grid(row=1, sticky=E)
self.entry_username.grid(row=0, column=1)
self.entry_password.grid(row=1, column=1)
self.logbtn = Button(self, text="Login", command=self._login_btn_clicked)
self.logbtn.grid(columnspan=2)
self.pack()
def _login_btn_clicked(self):
# print("Clicked")
username = self.entry_username.get()
password = self.entry_password.get()
# print(username, password)
if username == "ed" and password == "1":
self.label_username.grid_forget()
self.label_password.grid_forget()
self.entry_username.grid_forget()
self.entry_password.grid_forget()
self.logbtn.grid_forget()
self.pack_forget()
app = App(root)
if username == "nb" and password == "2":
self.label_username.grid_forget()
self.label_password.grid_forget()
self.entry_username.grid_forget()
self.entry_password.grid_forget()
self.logbtn.grid_forget()
self.pack_forget()
app = App(root)
root = Tk()
root.minsize(950, 450)
root.title("SIGN OFF LABELS")
lf = login(root)
# app = App(root)
root.mainloop()
1 Answer 1
NEVER use:
from tkinter import *
Instead, use:
import tkinter as tk
or even better:
from tkinter import Class1, Class2
This allows everyone reading your code (like the people answering your question) to easily see where what name is coming from, and makes it that much simpler for other people's input to come your way without having to ask clarifying questions where variable X is coming from.
I'll be restricting myself to your login() class, since I'm not experienced with tkinter. I am, however, experienced with (Py)Qt, so I can offer some advice.
Naming
In Python, we name classes with PascalCase (aka TitleCase). It's also a widget of some kind, so put that in the name as well, to make it easier. login() looks like a function. I'd think something like LoginWidget
or if it's more tkinter-like, LoginFrame
.
Widget/Frame destruction
Practically, you're getting rid of your Login widget when it's function's done. Why don't you just delete the widget and create any widget depending on what you need to login for? Typically, this should be done in the widget/frame's master
. If you kept all other references inside, you don't really have to think about deletion of it's instance variables if you just delete the entire thing.
Look up account data
Also DO change your _login_btn_clicked()
method to look up the login data from elsewhere. Even if it's just a file named passwords.txt where you save it in plain text. It's no more unsafe than storing it directly in your code.
Aside from that, it also avoids code repetition, like having the exact same result for two different accounts like you do.
Given a passwords.txt which stores them like username:password (Can't say to often, but this is VERY unsafe):
def _login_btn_clicked(self):
# print("Clicked")
username = self.entry_username.get()
password = self.entry_password.get()
# print(username, password)
account_list = [line.split(":", maxsplit=1) for line in open("passwords.txt")]
# list of 2-tuples. Usersnames with colons inside not supported.
accounts = {key: value.rstrip() for key, value in account_list}
# Convert to dict[username] = password, and slices off the line ending.
# Does not support passwords ending in whitespace.
if accounts[username] == password:
store_account_information_for_later_use()
inform_master_that_I_am_obsolete()
# app = App(root) # Should be done by self.master.
else:
notify_user_of_failure()
In the case that this login frame is a window instead of embedded into one, you can just close it an make a new window afterwards.
Please do note that some of my invented function names describe more verbose what they should do, and that these names are to long for normal function names.
-
\$\begingroup\$ hi, when you said #Dont know what it does here at
app = App(root)
this basically opens theApp()
class after logged in. I will read through your feedback when I have time and get back to it asap. Thanks for your time in answering! \$\endgroup\$DeveloperLV– DeveloperLV2019年09月27日 08:41:55 +00:00Commented Sep 27, 2019 at 8:41 -
\$\begingroup\$ Ah, ok. In that case, I'd put it in the same function/class that created the Login class itself. \$\endgroup\$Gloweye– Gloweye2019年09月27日 08:43:54 +00:00Commented Sep 27, 2019 at 8:43