HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Simple data entry form that writes data to textfile

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
entrysimplewritesthattextfileformdata

Problem

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 = []
t

Solution

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.

Code Snippets

import tkinter as tk
from tkinter import *
import tkinter as tk
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)
class MainPage(tk.Frame):
    def __init__(self, parent, controller):
        self.controller = controller
        ...

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

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

Context

StackExchange Code Review Q#108035, answer score: 6

Revisions (0)

No revisions yet.