3
\$\begingroup\$

I am making a basic program just for a local machine so I do not need to create anything fancy but I am finding that my code is going to become quite lengthy. Any tips on how to improve it so I am not repeating code? I have additional code to add in respect to client data etc but I am primarily concentrating on the foundation first.

from tkinter import *
from PIL import ImageTk, Image

root = Tk()
root.withdraw()


def login(event):
    if ent1.get() == 'admin' and ent2.get() == 'password':
        root.iconify()
        top.destroy()


def client_data(event):
    root.withdraw()
    top = Toplevel()
    top.title('Client Data')
    top.geometry('800x500')
    top.configure(background='grey')
    client1 = Message(top, text='Random skeleton', bg='grey', width=350)
    client1.pack()
    x = Button(top, text='Close', bg='red', command=top.destroy)
    root.iconify()
    x.pack()


image1 = ImageTk.PhotoImage(Image.open('ileye.png'))
top = Toplevel()
top.title('Login')
top.configure(background='grey')
photo = Label(top, image=image1)
photo.pack()
user = Label(top, text='User name', bg='grey')
user.pack()
ent1 = Entry(top, bg='grey')
ent1.pack()
pwd = Label(top, text='Password', bg='grey')
pwd.pack()
ent2 = Entry(top, show='*', bg='grey')
ent2.bind('<Return>', login)
ent2.pack()
ex = Button(top, text='EXIT', command=root.quit)
ex.pack()
check = Checkbutton(top, text='Remember me', bg='grey')
check.pack()


root.title('Main Screen')
root.attributes('-zoomed', True)
menu_drop = Menu(root)
root.config(menu=menu_drop)
file = Menu(menu_drop)
menu_drop.add_cascade(label='Options', menu=file)
file.add_command(label='New', command=root.quit)  ## New Function
client = Button(root, text='Client list')
file.add_separator()
file.add_command(label='Close App', command=root.quit)
client.bind('<Button-1>', client_data)
client.pack()
exi = Button(root, text='EXIT', command=root.quit)
exi.pack()


root.mainloop()
\$\endgroup\$
2
\$\begingroup\$

Any tips on how to improve it so I am not repeating code?

There isn't really a lot of repeated code, here. I think you're fine on that front.

Avoid globals

Starting with image1 onward, you have lots of globals. Try to move them into a class, for a few reasons: re-entrance (you could spawn multiple identical windows), general code organization, namespace cleanliness, etc.

Credential management

if ent1.get() == 'admin' and ent2.get() == 'password':

I know that you are

making a basic program just for a local machine

but this is never a good habit to get into. Hard-coding passwords is not secure, and it isn't much more difficult to do the "right" thing. Import a library to be able to securely handle authentication and encrypted credential storage.

\$\endgroup\$
  • \$\begingroup\$ Brill thanks for the advice. I will probably use crypto or something similar in respect to credential management. I will put the globals into a class also. I want it to be right \$\endgroup\$ – Barb Sep 23 at 13:08
1
\$\begingroup\$

Never do this:

from tkinter import *

Instead use:

from tkinter import Every, Class, I, Will, Ever, Need

Or if your really must:

import tkinter as tk
# Or:
import tkinter

or similar. import * just dumps all module-level variables in your global space, which makes it a lot harder to read your code.

GUI Organization

I'm not 100% on how much this applies to Tkinter, but for PyQt we'd put as much of the initialization of GUI elements into extensions of the element's classes. So any child widget of X would be defined by and inside a method of X.

Otherwise, you could put it all in a function named main() and then call that function inside a if __name__ == "__main__": guard.

\$\endgroup\$
  • \$\begingroup\$ Thats a very good point thank you for that! I will amend it \$\endgroup\$ – Barb Sep 23 at 13:08
  • \$\begingroup\$ "Never do this" - there's a little more nuance than that. Another alternative is to import tkinter as tk, so that you don't have to spell out all of your imported classes, and the module references are more brief. Also note that your advice forces people to fill their namespace with module symbols, which shouldn't always be done. \$\endgroup\$ – Reinderien Sep 23 at 13:36
  • \$\begingroup\$ I guess that's worth mentioning as alternative. Personally, though, I find that long.dotted.names also have a tendency to confuse me when parsing code. \$\endgroup\$ – Gloweye Sep 23 at 13:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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