8
\$\begingroup\$

I am also pretty new to Python so I want to show my code for a very small currency converter with an Tkinter GUI.

The code does what I want, but now I want to optimize it and find new "next level" topics for me to learn. So I would be happy if some of you could take a look on my code and please give me constructive criticism.

Many thanks!

from tkinter import *
import tkinter.ttk as ttk

def currency_converter():
    #creating instance of tkinter
    currency_converter = Tk()  
    #Set title of our window form  
    currency_converter.title("My First Tool - WOW")
    #Set dimension of form 
    currency_converter.geometry("425x225")
    #Centers the Window
    currency_converter.update_idletasks()
    w = currency_converter.winfo_screenwidth()
    h = currency_converter.winfo_screenheight()
    size = tuple(int(_) for _ in currency_converter.geometry().split('+')[0].split('x'))
    x = w/2 - size[0]/2
    y = h/2 - size[1]/2
    currency_converter.geometry("%dx%d+%d+%d" % (size + (x, y))) 

    currency_converter.rowconfigure(5, weight=1)

    currency_converter.lift()

#    currency_converter.overrideredirect(1) #Remove border
    currency_converter.configure(background='#007780')

    def enter(event):
        UserInput = float(Currency_Input.get().replace(',', '.'))
        Currency_Output.delete(0,END)
        Currency_Output1.delete(0,END)
        Currency_Output2.delete(0,END)
        if box.get() == "EUR":            
            Currency_Output_Label.config(text="USD")
            Currency_Output.insert(0,round(UserInput*1.237203,2))
            Currency_Output1_Label.config(text="GBP")
            Currency_Output1.insert(0,round(UserInput*0.863629637,2))
            Currency_Output2_Label.config(text="CNY")
            Currency_Output2.insert(0,round(UserInput*7.76364208,2))
        elif box.get() == "USD":
            Currency_Output_Label.config(text="EUR")
            Currency_Output.insert(0,round(UserInput*0.808274794,2))
            Currency_Output1_Label.config(text="GBP")
            Currency_Output1.insert(0,round(UserInput*0.698050067,2))
            Currency_Output2_Label.config(text="CNY")
            Currency_Output2.insert(0,round(UserInput*6.28594776,2))
        elif box.get() == "GBP":
            Currency_Output_Label.config(text="EUR")
            Currency_Output.insert(0,round(UserInput*1.15790376,2))
            Currency_Output1_Label.config(text="USD")
            Currency_Output1.insert(0,round(UserInput*1.432562,2))
            Currency_Output2_Label.config(text="CNY")
            Currency_Output2.insert(0,round(UserInput*9.0008486,2))

        elif box.get() == "CNY":
            Currency_Output_Label.config(text="EUR")
            Currency_Output.insert(0,round(UserInput*0.128805526,2))
            Currency_Output1_Label.config(text="USD")
            Currency_Output1.insert(0,round(UserInput*0.159085,2))
            Currency_Output2_Label.config(text="GBP")
            Currency_Output2.insert(0,round(UserInput*0.111100636,2))

    def close_currency_converter():
        currency_converter.destroy()

    Headline_Label = Label(currency_converter, text='Currency Converter', bg='#007780', fg='white',font=("Century Gothic",16))
    Headline_Label.grid(row=0,column=0, columnspan=2, padx=5, pady=5, sticky=W)

    Box_Headline_Label = Label(currency_converter, text='Which Currency?', bg='#007780', fg='white',font=("Century Gothic",11))
    Box_Headline_Label.grid(row=1,column=0, columnspan=1, padx=5, pady=5, sticky=W)

    box_value = StringVar() 
    box = ttk.Combobox(currency_converter, textvariable=box_value, width=10)
    box['values'] = ('EUR', 'USD', 'GBP', 'CNY')
    box.current(0)
    box.grid(row=1,column=1, pady=5, sticky=E)

    Currency_Input = Entry(currency_converter)
    Currency_Input.grid(row=1,column=2, padx=10, pady=5, sticky=W)

    Currency_Input.bind('<Return>',enter)

    Currency_Output_Label = Label(currency_converter, text='', bg='#007780', fg='white',font=("Century Gothic",11))
    Currency_Output_Label.grid(row=2,column=1, padx=5, pady=5, sticky=W)

    Currency_Output1_Label = Label(currency_converter, text='', bg='#007780', fg='white',font=("Century Gothic",11))
    Currency_Output1_Label.grid(row=3,column=1, padx=5, pady=5, sticky=W)   

    Currency_Output2_Label = Label(currency_converter, text='', bg='#007780', fg='white',font=("Century Gothic",11))
    Currency_Output2_Label.grid(row=4,column=1, padx=5, pady=5, sticky=W)  

    Currency_Output = Entry(currency_converter)
    Currency_Output.grid(row=2,column=2, padx=10, pady=5, sticky=W)

    Currency_Output1 = Entry(currency_converter)
    Currency_Output1.grid(row=3,column=2, padx=10, pady=5, sticky=W)

    Currency_Output2 = Entry(currency_converter)
    Currency_Output2.grid(row=4,column=2, padx=10, pady=5, sticky=W)

    Button(currency_converter,text="Quit",command=close_currency_converter).grid(row=6,column=0, sticky=E+S+W, pady=5, padx=5)

    currency_converter.mainloop()

currency_converter()
\$\endgroup\$
3
  • \$\begingroup\$ Is this running slowly and needs to be faster? If not you might consider dropping the performance tag; also add a beginner tag. \$\endgroup\$
    – yuri
    Apr 18, 2018 at 7:58
  • \$\begingroup\$ I think the performance is ok. I droped performance and added beginner to the tags. Thank you! \$\endgroup\$
    – K-Doe
    Apr 18, 2018 at 7:59
  • \$\begingroup\$ You may consider tkinter best practices \$\endgroup\$ Apr 29, 2018 at 13:06

1 Answer 1

5
\$\begingroup\$

pep-8

Try to follow the Python styleguide, and above all, be consistent. Now you use a mix between CamelCase, snake_case and Camel_Snake

imports

try to avoid from xxx import *. This pollutes the namespace and makes it unclear where certain variables or functions come from

hardcoding values

You hardcoded the relative values of the different currencies. What if after a few days the exchange rates differ? Then you'll have to track this manually over your whole file. Easiest would be to keep 1 dict with the relative ratios, or 1 dict with nested a dict per currency

something like this:

CURRENCIES = {
    'EUR': {
        'USD': xxxx,
        'GBP': xxxx,
        ...,
    },
    'USD': {
        'EUR': xxx,
        'GBP': xxx,
        ...
    },
    ...
}

The added benefit of this is you can use the keys of this dict to populate the comboboxes

main guard

put the calling of the main function behind if __name__ == '__main__:, so this code can be imported in other places too.

separate the logic

I would separate the calculation and the presentation.

calculation

The calculation can be just:

def convert(source, destination, amount):
    return amount * CURRENCIES[source][destination]

If you want, you can refactor this into a class, with the currencies passed into it as argument for the __init__

Presentation

For the GUI I would make a class, that gets populated in the __init__, where the values of the important input fields and comboboxes are properties, and pushing buttons or other actions trigger different functions

\$\endgroup\$
1
  • \$\begingroup\$ Thank you very much. Everything from you answer helped me a lot. I have just one request remaining: If you want, you can refactor this into a class, with the currencies passed into it as argument for the _init_ could you please provide me an example? I found some, but in didn't really get the idea of init, \$\endgroup\$
    – K-Doe
    Apr 19, 2018 at 12:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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