4
\$\begingroup\$

I made a data entry form that writes each category into a text file. As an example I used Regular, Premium and Diesel categories. They each have open, delivery, total, sales and close numbers for that day. How can I can improve my code? Also I tried to implement the login form but I couldn't get it to work.

import tkinter as tk
from tkinter import *


class Application(tk.Tk):
    def __init__(self, *args, **kwargs):
        tk.Tk.__init__(self, *args, **kwargs)
        window = tk.Frame(self, width=800, height=600)
        window.pack(side="top", fill="both", expand=True)
        window.grid_rowconfigure(0, weight=1)
        window.grid_columnconfigure(0, weight=1)
        self.frames = {}

        for F in (MainPage, DataEntryForm):
            frame = F(window, self)
            self.frames[F] = frame
            frame.grid(row=0, column=0, sticky="nsew")

        self.show_frame(MainPage)

    def show_frame(self, cont):
        frame = self.frames[cont]
        frame.tkraise()


class MainPage(tk.Frame):

    def __init__(self, parent, controller):
        tk.Frame.__init__(self, parent)

        labelun = tk.Label(self, text='Username')
        labelun.grid(row=0, column=0)
        labelpw = tk.Label(self, text='Password')
        labelpw.grid(row=1, column=0)

        uname_entry = tk.Entry(self, textvariable=StringVar())
        uname_entry.grid(row=0, column=1, columnspan=2)
        pword_entry = tk.Entry(self, textvariable=StringVar())
        pword_entry.grid(row=1, column=1, columnspan=2)

        butenter = tk.Button(self, text='Enter', padx=22, command=lambda: controller.show_frame(DataEntryForm))
        butenter.grid(row=2, column=1)
        butquit = tk.Button(self, text='Quit', padx=22, command=self.quit)
        butquit.grid(row=2, column=2)


class DataEntryForm(tk.Frame):

    def __init__(self, parent, controller):
        tk.Frame.__init__(self, parent)
        self.reglist = []
        self.prelist = []
        self.dieslist = []
        titlelabel = tk.Label(self, text='Sales Data Entry Form', font='Verdana, 16')
        titlelabel.grid(row=0, column=1, columnspan=4)
        datelabel = tk.Label(self, text='Enter Date', font='Verdana, 12')
        datelabel.grid(row=1, column=0)
        self.dateentry = tk.Entry(self, textvariable=StringVar())
        self.dateentry.grid(row=1, column=1, sticky='w')

        openlabel = tk.Label(self, text='Open', font='Verdana, 12')
        openlabel.grid(row=2, column=1)
        deliverylabel = tk.Label(self, text='Delivery', font='Verdana, 12')
        deliverylabel.grid(row=2, column=2)
        totallabel = tk.Label(self, text='Total', font='Verdana, 12')
        totallabel.grid(row=2, column=3)
        saleslabel = tk.Label(self, text='Sales', font='Verdana, 12')
        saleslabel.grid(row=2, column=4)
        closelabel = tk.Label(self, text='Close', font='Verdana, 12')
        closelabel.grid(row=2, column=5)
        reglabel = tk.Label(self, text='Regular', font='Verdana, 12', padx=0, pady=4)
        reglabel.grid(row=3, column=0, sticky='e')
        prelabel = tk.Label(self, text='Premium', font='Verdana, 12', padx=0, pady=4)
        prelabel.grid(row=4, column=0, sticky='e')
        dieslabel = tk.Label(self, text='Diesel', font='Verdana, 12', padx=0, pady=4)
        dieslabel.grid(row=5, column=0, sticky='e')

        self.regopenentry = tk.Entry(self, textvariable=StringVar())
        self.regopenentry.grid(row=3, column=1, padx=3, pady=4)
        self.regdeliveryentry = tk.Entry(self, textvariable=StringVar())
        self.regdeliveryentry.grid(row=3, column=2, padx=3, pady=4)
        self.regtotalentry = tk.Entry(self, textvariable=StringVar())
        self.regtotalentry.grid(row=3, column=3, padx=3, pady=4)
        self.regsalesentry = tk.Entry(self, textvariable=StringVar())
        self.regsalesentry.grid(row=3, column=4, padx=3, pady=4)
        self.regcloseentry = tk.Entry(self, textvariable=StringVar())
        self.regcloseentry.grid(row=3, column=5, padx=3, pady=4)

        self.preopenentry = tk.Entry(self, textvariable=StringVar())
        self.preopenentry.grid(row=4, column=1, padx=3, pady=4)
        self.predeliveryentry = tk.Entry(self, textvariable=StringVar())
        self.predeliveryentry.grid(row=4, column=2, padx=3, pady=4)
        self.pretotalentry = tk.Entry(self, textvariable=StringVar())
        self.pretotalentry.grid(row=4, column=3, padx=3, pady=4)
        self.presalesentry = tk.Entry(self, textvariable=StringVar())
        self.presalesentry.grid(row=4, column=4, padx=3, pady=4)
        self.precloseentry = tk.Entry(self, textvariable=StringVar())
        self.precloseentry.grid(row=4, column=5, padx=3, pady=4)

        self.diesopenentry = tk.Entry(self, textvariable=StringVar())
        self.diesopenentry.grid(row=5, column=1, padx=3, pady=4)
        self.diesdeliveryentry = tk.Entry(self, textvariable=StringVar())
        self.diesdeliveryentry.grid(row=5, column=2, padx=3, pady=4)
        self.diestotalentry = tk.Entry(self, textvariable=StringVar())
        self.diestotalentry.grid(row=5, column=3, padx=3, pady=4)
        self.diessalesentry = tk.Entry(self, textvariable=StringVar())
        self.diessalesentry.grid(row=5, column=4, padx=3, pady=4)
        self.diescloseentry = tk.Entry(self, textvariable=StringVar())
        self.diescloseentry.grid(row=5, column=5, padx=3, pady=4)

        butsavedata = tk.Button(self, text='Save', width=10, command=self.savedata)
        butsavedata.grid(row=6, column=5)
        butquit = tk.Button(self, text='Quit', width=10, command=self.quit)
        butquit.grid(row=6, column=4)

    def savedata(self):
        regulardata = self.regopenentry.get()
        self.reglist.append(regulardata)
        regulardata = self.regdeliveryentry.get()
        self.reglist.append(regulardata)
        regulardata = self.regtotalentry.get()
        self.reglist.append(regulardata)
        regulardata = self.regsalesentry.get()
        self.reglist.append(regulardata)
        regulardata = self.regcloseentry.get()
        self.reglist.append(regulardata)
        premiumdata = self.preopenentry.get()
        self.prelist.append(premiumdata)
        premiumdata = self.predeliveryentry.get()
        self.prelist.append(premiumdata)
        premiumdata = self.pretotalentry.get()
        self.prelist.append(premiumdata)
        premiumdata = self.presalesentry.get()
        self.prelist.append(premiumdata)
        premiumdata = self.precloseentry.get()
        self.prelist.append(premiumdata)
        dieseldata = self.diesopenentry.get()
        self.dieslist.append(dieseldata)
        dieseldata = self.diesdeliveryentry.get()
        self.dieslist.append(dieseldata)
        dieseldata = self.diestotalentry.get()
        self.dieslist.append(dieseldata)
        dieseldata = self.diessalesentry.get()
        self.dieslist.append(dieseldata)
        dieseldata = self.diescloseentry.get()
        self.dieslist.append(dieseldata)
        dateentered = self.dateentry.get()
        open('RegularData.txt', 'a').write(dateentered + '   ' + '   '.join([str(i) for i in self.reglist]) + '\n')
        open('PremiumData.txt', 'a').write(dateentered + '   ' + '   '.join([str(i) for i in self.reglist]) + '\n')
        open('DieselData.txt', 'a').write(dateentered + '   ' + '   '.join([str(i) for i in self.reglist]) + '\n')
        self.reglist = []
        self.prelist = []
        self.dieslist = []


my_gui = Application()
my_gui.mainloop()
\$\endgroup\$
6
\$\begingroup\$

Importing tkinter

Pick a single way to import tkinter. You currently have this:

import tkinter as tk
from tkinter import *

Remove the second line so that you only have this:

import tkinter as tk

That means that you have to prefix all tkinter classes and constants with tk., which is a Good Thing. The Zen of Python says explicit is better than implicit, and PEP8 says to avoid global imports.

Group your layout commands together

You have a pattern where you create a widget, call grid on the widget, create a widget, call grid on the widget, etc. This makes it heard to visualize the layout when looking at the code. Group all of your layout together.

For example:

def __init__(self, parent, controller):
    tk.Frame.__init__(self, parent)

    labelun = tk.Label(self, text='Username')
    labelpw = tk.Label(self, text='Password')
    uname_entry = tk.Entry(self, textvariable=StringVar())
    pword_entry = tk.Entry(self, textvariable=StringVar())
    butenter = tk.Button(self, text='Enter', padx=22, command=lambda: controller.show_frame(DataEntryForm))
    butquit = tk.Button(self, text='Quit', padx=22, command=self.quit)

    labelun.grid(row=0, column=0)
    labelpw.grid(row=1, column=0)
    uname_entry.grid(row=0, column=1, columnspan=2)
    pword_entry.grid(row=1, column=1, columnspan=2)
    butenter.grid(row=2, column=1)
    butquit.grid(row=2, column=2)

Avoid lambda unless it's absolutely necessary

Don't use lambda unless it's absolutely necessary. It makes your code harder to write, harder to understand, and harder to debug. To fix this, make sure you save the controller as an attribute on each page:

class MainPage(tk.Frame):
    def __init__(self, parent, controller):
        self.controller = controller
        ...

class DataEntryForm(tk.Frame):
    def __init__(self, parent, controller):
        self.controller = controller
        ...

Next, change your button to call a function designed specifically for that button:

class MainPage(tk.Frame):
    def __init__(...):
        butenter = tk.Button(..., command=self.on_enter, ...)

    def on_enter(self):
        self.controller.show_frame(DataEntryForm)

This may seem like overkill when your function only calls a single function, but this is a good pattern to be in the habit of following. Eventually you may want some logging. or some error checking, or form validation.

Note: some people like to prefix callbacks with "on_" to make it easier to spot callbacks. Other people prefix it with a simple underscore. What you name it isn't particularly important as long as you're consistent.

Reduce repetitive code, and optimize for clarity

Consider this block of code:

regulardata = self.regopenentry.get()
self.reglist.append(regulardata)
regulardata = self.regdeliveryentry.get()
self.reglist.append(regulardata)
regulardata = self.regtotalentry.get()
self.reglist.append(regulardata)
regulardata = self.regsalesentry.get()
self.reglist.append(regulardata)
regulardata = self.regcloseentry.get()
self.reglist.append(regulardata)

There's really no reason to use an intermediate value (regulardata) if you're immediately appending the data to a list. You also have the potential for a bug, since this function assumes that the lists are empty (which they are now, but bugs in other parts of your code might forget to reset these lists).

You can rewrite it like this:

self.reglist = [
    self.regopenentry.get(),
    self.regdeliveryentry.get(),
    self.regtotalentry.get(),
    self.regsalesentry.get(),
    self.regcloseentry.get(),
]

IMO that is much easier to read, and guarantees that the list always has exactly 5 entries.

\$\endgroup\$
2
\$\begingroup\$

Why are you doing both of these?

import tkinter as tk
from tkinter import *

from module import * is frequently advised against. The reason is that I know can't tell what might be coming from that function. Maybe tkinter has a print function that will override the inbuilt one and you'd never realise it if you weren't familiar with all the functions contained inside. I have heard that tkinter might suggest you use this syntax, but if you're going to import * then why are you also importing plainly? You should stick to one or the other, not do both together. In case you're confused, you can access everything you need from tkinter with just your first line. Like this:

import tkinter as tk

test = tk.someclass(tk.some_value)
tk.somefunction(test)

Using from tkinter import * is a way to avoid having to use the tk. prefix. So it could look like this instead:

from tkinter import *

test = someclass(some_value)
somefunction(test)
\$\endgroup\$

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.