6
\$\begingroup\$

How can I improve the code? Specifically, is there any way that I can move the code from App.handle into separate functions?

from tkinter import *
import operator

# tuple of buttons in the calculator
# string = command
# function = operation
# number = number input
BUTTONS = (
    (('E', 'exit'), ('<-', 'backspace'), ('CE', 'clear'), ('/', operator.truediv)),
    (('7', 7), ('8', 8), ('9', 9), ('*', operator.mul)),
    (('4', 4), ('5', 5), ('6', 6), ('-', operator.sub)),
    (('1', 1), ('2', 2), ('3', 3), ('+', operator.add)),
    (('.', 'decimal'), ('0', 0), ('^', operator.pow), ('=', 'evaluate'))
)

# transpose the list
BUTTONS = zip(*BUTTONS)

def get_font(size):
    """
    Return a font tuple
    """
    return ('Verdana', size)

class App(Frame):
    def __init__(self, parent):
        super().__init__(parent)
        self.pack()

        self.top_bar = Frame(self)
        self.top_bar.pack(fill=X, side=TOP, pady=20)

        self.big_num = Label(self.top_bar, text='', font=get_font(32))
        self.big_num.pack(fill=BOTH, expand=True)

        self.reset_calc()

        self.button_container = Frame()
        self.button_container.pack(side=BOTTOM, expand=True, fill=BOTH)
        for column in BUTTONS:
            frame = Frame(self.button_container)
            frame.pack(fill=BOTH, expand=True, side=LEFT)
            for item in column:
                button = Button(frame, text=item[0], font=get_font(11),
                                command=lambda x=item[1]: self.handle(x))
                button.pack(fill=BOTH, expand=True, side=TOP)

    def reset_calc(self):
        """
        Reset everything.
        """
        self.operator_function = None
        self.first_number = None
        self.set_text('')
        self.displaying_solution = False

    # 3 functions to modify calculator bar text
    def get_text(self):
        return self.big_num.cget('text')

    def set_text(self, text):
        self.big_num.config(text=text)

    def append_text(self, text):
        self.set_text(self.get_text() + text)

    def handle(self, var):
        """
        Handle button clicks.
        var = string, number, or function that is bound to the button.
        """
        if isinstance(var, str):
            if var == 'exit':
                root.quit()
            elif var == 'clear':
                self.reset_calc()
            elif var == 'decimal':
                if self.displaying_solution:
                    self.reset_calc()
                self.append_text('.')
            elif var == 'backspace':
                self.set_text(self.get_text()[:-1])
            elif var == 'evaluate':
                if self.operator_function is not None and \
                        (not self.displaying_solution) and \
                        self.first_number is not None:
                    text = self.get_text()
                    try:
                        num = float(text)
                    except ValueError:
                        pass
                    else:
                        solution = self.operator_function(self.first_number,
                                                          num)
                        if int(solution) == solution:
                            solution = int(solution)
                        else:
                            solution = round(solution, 9)
                        self.set_text(str(solution))
                        self.displaying_solution = True
        elif isinstance(var, int):
            if self.displaying_solution:
                self.reset_calc()
            self.append_text(str(var))
        elif hasattr(var, '__call__'):
            # if var is a function
            if not self.displaying_solution:
                self.operator_function = var
                text = self.get_text()
                try:
                    num = float(text)
                except ValueError:
                    pass
                else:
                    self.first_number = num
                    self.set_text('')

def main():
    root = Tk()
    root.geometry('400x400')
    root.title('Calculator')
    app = App(root)
    app.mainloop()

if __name__ == '__main__':
    main()
\$\endgroup\$
2
  • \$\begingroup\$ You didn't treat the division by 0 case; You also didn't treat the case where the user will press on E. You'll most probably get a NameError. I don't have time to write an answer, so I'll let somebody else to talk about these cases \$\endgroup\$ Commented Apr 18, 2017 at 6:49
  • \$\begingroup\$ root is not global, that means that pressing E button results in an error and the app doesn't quit. \$\endgroup\$
    – ChatterOne
    Commented Apr 18, 2017 at 8:57

1 Answer 1

2
\$\begingroup\$

There are a lot of areas where you could improve your code, but I'm going to focus on simplifying the handlers.


Connect your buttons to what they do

It's generally good practice to tie your buttons to their respective handlers. That way you can see what button does what while also allowing your code to be more freely modified (buttons getting added, changed, or removed). Here's a basic idea of what I mean:

def create_button(key, handler):
    return {
        key: key,
        handler: handler
    }

BUTTONS = {
  exit: create_button('E', App.exit),
  backspace: create_button('<-', App.backspace),
  clear: create_button('cl', App.reset_calc),
  # ...

  0: create_button('0', lambda app: App.input(app, 0)),
  1: create_button('1', lambda app: App.input(app, 1)),
  2: create_button('2', lambda app: App.input(app, 2)),
  # ...
}

LAYOUT = (
    (BUTTONS.exit, BUTTONS.backspace, BUTTONS.clear, BUTTONS.div),
    (BUTTONS[7], BUTTONS[8], BUTTONS[9], BUTTONS.mul),
    (BUTTONS[4], BUTTONS[5], BUTTONS[6], BUTTONS.sub),
    (BUTTONS[1], BUTTONS[2], BUTTONS[3], BUTTONS.add),
    (BUTTONS[1].decimal, BUTTONS[0], BUTTONS.pow, BUTTONS.evalaute)
)

This way new buttons can be added to the BUTTONS dictionary, and if you want to change the LAYOUT, you can easily switch which buttons go where.

It also comes with the added benefit that you'll be able to move your code in App.handle into seperate functions and won't need to do all the checks: if is a string or if is an int or is equal to 'exit' or .... All you need is the following change to the App constructor:

for item in column:
    button = Button(frame, text=item.key, font=get_font(11),
        command=lambda h=item.handle: h(self))

Breaking up the handle method

Add the required functionality to the App class so that the button's handlers will be able to access it:

class App(Frame):
    def __init__(self, parent):
        super().__init__(parent)
        self.parent = parent # ChatterOne commented on this for exit
        # ...
    # ...
    def exit(self):
        self.parent.quit()

    def input(self, var):
        if self.displaying_solution:
            self.reset_calc()
        self.append_text(str(var))
\$\endgroup\$

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.