3
\$\begingroup\$

Could I please have some ideas how to improve this conversion calculator I have made. I am currently not looking to add any more quantities, but rather trying to enhance the usability and add some quality of life changes to it.

If there are any major ways I could make the program more efficient that would be helpful as well.

Here is my code:

# Conversion Calculator #
#rid input unit 
from tkinter import *
from tkinter import ttk

class GUI():

    def __init__(self, master):

        self.master = master
        master.resizable(False, False)
        master.title('Conversion Calculator')

        self.tabControl = ttk.Notebook(master) # Tab Formatting
        self.tab1 = ttk.Frame(self.tabControl)
        self.tabControl.add(self.tab1, text='Weight')
        self.tab2 = ttk.Frame(self.tabControl)
        self.tabControl.add(self.tab2, text='Length')
        self.tab3 = ttk.Frame(self.tabControl)
        self.tabControl.add(self.tab3, text='Tempurature')
        self.tabControl.pack(expand=1, fill="both")

        self.intial_wording11 = StringVar(self.tab1) # Tab 1 Formatting
        self.intial_wording11.set('Unit')
        self.intial_wording12 = StringVar(self.tab1)
        self.intial_wording12.set('Unit')
        self.entrybox1 = ttk.Entry(self.tab1, width=12)
        self.entrybox1.grid(column=0, row=1)
        self.entrybox1.focus_set()
        self.Iunit1 = StringVar()
        self.input_unit1 = ttk.Combobox(self.tab1, width=12, textvariable=self.Iunit1, state="readonly", values = ['Unit', 'Grams', 'Kilograms', 'Tons', 'Ounces', 'Pounds'])
        self.input_unit1.grid(column=1, row=1)
        self.input_unit1.current(0)
        self.convert_bttn1 = ttk.Button(self.tab1, text='Convert', command=self.weight)
        self.convert_bttn1.grid(column=0, row=3)
        self.outputbox1 = ttk.Entry(self.tab1, width=12, state="readonly")
        self.outputbox1.grid(column=0, row=4)
        self.Ounit1 = StringVar()
        self.output_unit1 = ttk.Combobox(self.tab1, width=12, textvariable=self.Ounit1, state="readonly", values = ['Unit', 'Grams', 'Kilograms', 'Tons', 'Ounces', 'Pounds'])
        self.output_unit1.grid(column=1, row=4)
        self.output_unit1.current(0)

        self.intial_wording21 = StringVar(self.tab2) # self.tab 2 Formatting #
        self.intial_wording21.set('Unit')
        self.intial_wording22 = StringVar(self.tab2)
        self.intial_wording22.set('Unit')
        self.entrybox2 = ttk.Entry(self.tab2, width=12)
        self.entrybox2.grid(column=0, row=1)
        self.entrybox2.focus_set()
        self.Iunit2 = StringVar()
        self.input_unit2 = ttk.Combobox(self.tab2, width=12, textvariable=self.Iunit2, state="readonly", values = ['Unit', 'Millimeters', 'Centimeters', 'Meters', 'Kilometers', 'Miles', 'Feet', 'Inches', 'Yards'])
        self.input_unit2.grid(column=1, row=1)
        self.input_unit2.current(0)
        self.convert_bttn2 = ttk.Button(self.tab2, text='Convert', command=self.length)
        self.convert_bttn2.grid(column=0, row=3)
        self.outputbox2 = ttk.Entry(self.tab2, width=12, state="readonly")
        self.outputbox2.grid(column=0, row=4)
        self.Ounit2 = StringVar()
        self.output_unit2 = ttk.Combobox(self.tab2, width=12, textvariable=self.Ounit2, state="readonly", values = ['Unit', 'Millimeters', 'Centimeters', 'Meters', 'Kilometers', 'Miles', 'Feet', 'Inches', 'Yards'])
        self.output_unit2.grid(column=1, row=4)
        self.output_unit2.current(0)

        self.intial_wording31 = StringVar(self.tab3) # self.tab 3 Formatting #
        self.intial_wording31.set('Unit')
        self.intial_wording32 = StringVar(self.tab3)
        self.intial_wording32.set('Unit')
        self.entrybox3 = ttk.Entry(self.tab3, width=12)
        self.entrybox3.grid(column=0, row=1)
        self.entrybox1.focus_set()
        self.Iunit3 = StringVar()
        self.input_unit3 = ttk.Combobox(self.tab3, width=12, textvariable=self.Iunit3, state="readonly", values = ['Unit', 'Celcius', 'Farenheit', 'Kelvin']) # self.intial_wording
        self.input_unit3.grid(column=1, row=1)
        self.input_unit3.current(0)
        self.convert_bttn3 = ttk.Button(self.tab3, text='Convert', command=self.tempurature)
        self.convert_bttn3.grid(column=0, row=3)
        self.outputbox3 = ttk.Entry(self.tab3, width=12, state="readonly")
        self.outputbox3.grid(column=0, row=4)
        self.Ounit3 = StringVar()
        self.output_unit3 = ttk.Combobox(self.tab3, width=12, textvariable=self.Ounit3, state="readonly", values = ['Unit', 'Celcius', 'Farenheit', 'Kelvin'])#self.intial_wording2, 
        self.output_unit3.grid(column=1, row=4)
        self.output_unit3.current(0)

        self.master.bind('<Return>', self.keybind)

class Logic(GUI):

    def keybind (self, event):
        self.convert_bttn1.invoke()
        self.convert_bttn2.invoke()
        self.convert_bttn3.invoke()


    def weight(self):
        Weight_Conversion_Values = {'Grams': 1,
                                    'Kilograms': 1000,
                                    'Tons': 1000000, 
                                    'Ounces': 28.3495,
                                    'Pounds': 453.592}
        self.outputbox1.delete(0, END)
        if self.Iunit1.get() != '':
            if self.Ounit1.get() != '':
                try:
                    self.outputbox1.config(state='NORMAL')
                    self.outputbox1.delete(0, 'end') 
                    self.outputbox1.config(state='readonly')
                    text = float(self.entrybox1.get())
                    newtext = text*(Weight_Conversion_Values[self.Iunit1.get()])*(int(1)/(Weight_Conversion_Values[self.Ounit1.get()]))
                    self.outputbox1.config(state='NORMAL')
                    self.outputbox1.insert(0, newtext)
                    self.outputbox1.config(state='readonly')
                except ValueError:
                    error_message = 'non-int error'
                    self.outputbox1.config(state='NORMAL')
                    self.outputbox1.insert(0, error_message)
                    self.outputbox1.config(state='readonly')

    def length(self):
        Length_Conversion_Values = {'Meters': 1,
                                    'Kilometers': 1000,
                                    'Centimeters': 0.01,
                                    'Millimeters': 0.001,
                                    'Miles': 1609.344498,
                                    'Feet': 0.304799999,
                                    'Inches': 0.02539998,
                                    'Yards': 0.9144027578}
        self.outputbox2.delete(0, END)
        if self.Iunit2.get() != '':
            if self.Ounit2.get() != '':
                try:
                    self.outputbox2.config(state='NORMAL')
                    self.outputbox2.delete(0, 'end') 
                    self.outputbox2.config(state='readonly')
                    text = float(self.entrybox2.get())
                    newtext = text*(Length_Conversion_Values[self.Iunit2.get()])*(int(1)/(Length_Conversion_Values[self.Ounit2.get()]))
                    self.outputbox2.config(state='NORMAL')
                    self.outputbox2.insert(0, newtext)
                    self.outputbox2.config(state='readonly')
                except ValueError:
                    error_message = 'non-int error'
                    self.outputbox2.config(state='NORMAL')
                    self.outputbox2.insert(0, error_message)
                    self.outputbox2.config(state='readonly')

    def tempurature(self):
        self.outputbox3.delete(0, END)
        if self.Iunit3.get() == 'Celcius':
            self.outputbox3.config(state='NORMAL')
            self.outputbox3.delete(0, 'end') 
            self.outputbox3.config(state='readonly')
            try:
                if self.Ounit3.get() == 'Kelvin':
                    if float(self.entrybox3.get()) >= -273.15:
                        text = float(self.entrybox3.get())
                        newtext = text + 273.15
                        self.outputbox3.config(state='NORMAL')
                        self.outputbox3.insert(0, newtext)
                        self.outputbox3.config(state='readonly')
                    else:
                        error_message = 'X ≥ -273.15'
                        self.outputbox3.config(state='NORMAL')
                        self.outputbox3.insert(0, error_message)
                        self.outputbox3.config(state='readonly')
                if self.Ounit3.get() == 'Farenheit':
                    if float(self.entrybox3.get()) >= -273.15:
                        text = float(self.entrybox3.get())
                        newtext = text*9/5 + 32
                        self.outputbox3.config(state='NORMAL')
                        self.outputbox3.insert(0, newtext)
                        self.outputbox3.config(state='readonly')
                    else:
                        error_message = 'X ≥ -273.15'
                        self.outputbox3.config(state='NORMAL')
                        self.outputbox3.insert(0, error_message)
                        self.outputbox3.config(state='readonly')
            except ValueError:
                    error_message = 'non-int error'
                    self.outputbox3.config(state='NORMAL')
                    self.outputbox3.insert(0, error_message)
                    self.outputbox3.config(state='readonly')
        if self.Iunit3.get() == 'Farenheit':
            self.outputbox3.config(state='NORMAL')
            self.outputbox3.delete(0, 'end') 
            self.outputbox3.config(state='readonly')
            try:
                if self.Ounit3.get() == 'Kelvin':
                    if float(self.entrybox3.get()) >= -459.67:
                        text = float(self.entrybox3.get())
                        newtext = (text+459.67)*5/9
                        self.outputbox3.config(state='NORMAL')
                        self.outputbox3.insert(0, newtext)
                        self.outputbox3.config(state='readonly')
                    else:
                        error_message = 'X ≥ -459.67'
                        self.outputbox3.config(state='NORMAL')
                        self.outputbox3.insert(0, error_message)
                        self.outputbox3.config(state='readonly')
                if self.Ounit3.get() == 'Celcius':
                    if float(self.entrybox3.get()) >= -459.67:
                        text = float(self.entrybox3.get())
                        newtext = text*5/9 - 32
                        self.outputbox3.config(state='NORMAL')
                        self.outputbox3.insert(0, newtext)
                        self.outputbox3.config(state='readonly')
                    else:
                        error_message = 'X ≥ -459.67'
                        self.outputbox3.config(state='NORMAL')
                        self.outputbox3.insert(0, error_message)
                        self.outputbox3.config(state='readonly')
            except ValueError:
                    error_message = 'non-int error'
                    self.outputbox3.config(state='NORMAL')
                    self.outputbox3.insert(0, error_message)
                    self.outputbox3.config(state='readonly')
        if self.Iunit3.get() == 'Kelvin':
            self.outputbox3.config(state='NORMAL')
            self.outputbox3.delete(0, 'end') 
            self.outputbox3.config(state='readonly')
            try:
                if self.Ounit3.get() == 'Celcius':
                    if float(self.entrybox3.get()) >= 0:
                        text = float(self.entrybox3.get())
                        newtext = text - 273.15
                        self.outputbox3.config(state='NORMAL')
                        self.outputbox3.insert(0, newtext)
                        self.outputbox3.config(state='readonly')
                    else:
                        error_message = 'X ≥ 0'
                        self.outputbox3.config(state='NORMAL')
                        self.outputbox3.insert(0, error_message)
                        self.outputbox3.config(state='readonly')
                if self.Ounit3.get() == 'Farenheit':
                    if float(self.entrybox3.get()) >= 0:
                        text = float(self.entrybox3.get())
                        newtext = text*9/5 - 459.67
                        self.outputbox3.config(state='NORMAL')
                        self.outputbox3.insert(0, newtext)
                        self.outputbox3.config(state='readonly')
                    else:
                        error_message = 'X ≥ 0'
                        self.outputbox3.config(state='NORMAL')
                        self.outputbox3.insert(0, error_message)
                        self.outputbox3.config(state='readonly')
            except ValueError:
                    error_message = 'non-int error'
                    self.outputbox3.config(state='NORMAL')
                    self.outputbox3.insert(0, error_message)
                    self.outputbox3.config(state='readonly')

if __name__ == "__main__":
    root = Tk()
    test = Logic(root)
\$\endgroup\$
4
\$\begingroup\$

One of the smartest programmer's I've ever known once told me that code is read way more often than it is written, so optimize for the reader1

While it's true that code should be efficient, it also needs to be understandable and maintainable. Also, efficiency is relative. If you're coding a GUI, a function that runs from a button click only needs to run faster than the eye can perceive (which is a long time in computer terms).

Most of my comments, then, are related to making the code more readable. That makes it more maintainable, and thus less prone to having bugs.


1 Optimize for the reader is, of course, a very general guideline. Some software needs to be more performant than others. The type of optimizations you do will be different for homework than for a 3D VR application, or a spacecraft orbiting venus, or a heart monitor, or a nuclear reactor.


Don't use wildcard imports

Use import tkinter as tk and then prefix all tk classes and commands with tk. (eg: tk.Tk(), tk.Frame(...), etc).

PEP8 discourages wildcard imports, and for good reasons. They pollute the global namespace, and they can overwrite variables and classes without you knowing. Tkinter is particularly susceptible to this since both tk and ttk define classes with the same name.

Group similar statements together

You should separate widget creation from widget layout. By grouping your layout code together it's much easier to visualize, and much easier to modify. It's also easier to notice inconsitencies, such as using slightly different colors or paddings for some widgets. My experience tells me that during development the layout can change fairly often.

Organize them in the same groups as they appear in the UI. For example, if all of your labels and entries are in one big block in the UI, make them one big block in the code. If you break them out into separate UI sections (eg: an address block, a name block, etc.) then organize them the code in the same way.

Also, create all of your variables at the same time so that it's easier to see:

For example, instead of this:

    self.entrybox1 = ttk.Entry(frame, width=12)
    self.entrybox1.grid(column=0, row=1)
    self.entrybox1.focus_set()
    self.Iunit1 = StringVar()
    self.input_unit1 = ttk.Combobox(self.tab1, width=12, textvariable=self.Iunit1, state="readonly", values = ['Unit', 'Grams', 'Kilograms', 'Tons', 'Ounces', 'Pounds'])
    self.input_unit1.grid(column=1, row=1)
    self.input_unit1.current(0)
    self.convert_bttn1 = ttk.Button(self.tab1, text='Convert', command=self.weight)
    self.convert_bttn1.grid(column=0, row=3)
    self.outputbox1 = ttk.Entry(self.tab1, width=12, state="readonly")
    self.outputbox1.grid(column=0, row=4)
    self.Ounit1 = StringVar()
    self.output_unit1 = ttk.Combobox(self.tab1, width=12, textvariable=self.Ounit1, state="readonly", values = ['Unit', 'Grams', 'Kilograms', 'Tons', 'Ounces', 'Pounds'])
    self.output_unit1.grid(column=1, row=4)
    self.output_unit1.current(0)

Do it like this:

self.Iunit1 = StringVar()
self.Ounit1 = StringVar()

self.entrybox1 = ttk.Entry(frame, width=12)
self.input_unit1 = ttk.Combobox(self.tab1, width=12, textvariable=self.Iunit1, state="readonly", values = ['Unit', 'Grams', 'Kilograms', 'Tons', 'Ounces', 'Pounds'])
self.convert_bttn1 = ttk.Button(self.tab1, text='Convert', command=self.weight)
self.outputbox1 = ttk.Entry(self.tab1, width=12, state="readonly")
self.output_unit1 = ttk.Combobox(self.tab1, width=12, textvariable=self.Ounit1, state="readonly", values = ['Unit', 'Grams', 'Kilograms', 'Tons', 'Ounces', 'Pounds'])

self.entrybox1.grid(column=0, row=1)
self.input_unit1.grid(column=1, row=1)
self.convert_bttn1.grid(column=0, row=3)
self.outputbox1.grid(column=0, row=4)
self.output_unit1.grid(column=1, row=4)

self.entrybox1.focus_set()
self.input_unit1.current(0)

Keep line lengths reasonable

Long lines are hard to read. Try to keep them shorter.

For example, instead of this:

self.input_unit1 = ttk.Combobox(self.tab1, width=12, textvariable=self.Iunit1, state="readonly", values = ['Unit', 'Grams', 'Kilograms', 'Tons', 'Ounces', 'Pounds'])

do this:

self.input_unit1 = ttk.Combobox(
    self.tab1, width=12, textvariable=self.Iunit1, state="readonly",
    values = ['Unit', 'Grams', 'Kilograms', 'Tons', 'Ounces', 'Pounds']
)

Remove dead code

You seem to have a lot of code that doesn't do anything. It should be removed.

self.intial_wording11 = StringVar(self.tab1) # Tab 1 Formatting
self.intial_wording11.set('Unit')
self.intial_wording12 = StringVar(self.tab1)
self.intial_wording12.set('Unit')
...
self.intial_wording21 = StringVar(self.tab2) # self.tab 2 Formatting #
self.intial_wording21.set('Unit')
self.intial_wording22 = StringVar(self.tab2)
self.intial_wording22.set('Unit')
...
self.intial_wording31 = StringVar(self.tab3) # self.tab 3 Formatting #
self.intial_wording31.set('Unit')
self.intial_wording32 = StringVar(self.tab3)
self.intial_wording32.set('Unit')

Don't use nested if statements if you only have one block of code

Instead of this:

if self.Iunit1.get() != '':
    if self.Ounit1.get() != '':
        ...

Do this:

if self.Iunit1.get() != '' and self.Ounit1.get() != '':
    ...

When you use nested if statements, it implies that you are going to have multiple conditions inside the first if. When reading the code, your brain makes a placeholder for additional conditions, but since there are none your brain has to think a bit harder than necessary to understand the code.

Use meaningful temporary variables

Consider this code:

if self.Iunit1.get() != '':
    ...
    newtext = text*(Weight_Conversion_Values[self.Iunit1.get()])*...)

You're calling self.Inunit1.get() twice, and you are also calling self.Inunit2.get() twice. Call it once, and store it in a meaningful variable.

For example:

input_unit = self.Iunit1.get()
output_unit = self.Iunit1.get()
...
newtext = text*(Weight_Conversion_Values[input_unit])*(int(1)/(Weight_Conversion_Values[output_unit]))

This reduces redundancy, and makes the expression easier to read.

Optimize for Readability

Weight_Conversion_Values is longer than it needs to be, making it's use hard to read. For example, you might call it "FACTOR" or "conversionFactor". Also, int(1) is redundant; 1 is already an integer:

newtext = text * (conversionFactor[input_unit]) * (1/(conversionFactor[output_unit]))

Notice how much more readable the above is compared to your original:

newtext = text*(Weight_Conversion_Values[self.Iunit1.get()])*(int(1)/(Weight_Conversion_Values[self.Ounit1.get()]))

Don't remove the ability for the user to resize the window

There's rarely a good reason to do this. Design your GUI so that it behaves properly when resized. You never know when a user might have a different resolution, different fonts, poor eyesight, or maybe they just want to shrink the window to fit in a small space. The user should be able to control their own windows.

Consider using functions to define each notebook tab.

Instead of having one big block of code to define the entire UI, break it into smaller chunks.

For example:

def __init__(self, master):

    self.master = master
    master.title('Conversion Calculator')

    self.tabControl = ttk.Notebook(master) # Tab Formatting
    self.tab1 = self.create_weight_tab()
    self.tab2 = self.create_length_tab()
    self.tab3 = self.create_temperature_tab()

    self.tabControl.add(self.tab1, text='Weight')
    self.tabControl.add(self.tab2, text='Length')
    self.tabControl.add(self.tab3, text='Tempurature')

def create_weight_tab(self):
    frame = ttk.Frame(self.tabControl)
    self.entrybox1 = ttk.Entry(frame, width=12)
    ...
    return frame
\$\endgroup\$
  • \$\begingroup\$ Really Appreciate this mate! I'm gonna have a deeper read later but until then thanks a lot! \$\endgroup\$ – copa Jun 30 '18 at 0:27

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.