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()
200_success
146k22 gold badges190 silver badges479 bronze badges
asked Apr 17, 2017 at 18:38
\$\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\$ 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))
answered Apr 25, 2017 at 7:30
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.