Issues
Background colors
Traceback (most recent call last): File "/usr/lib/python3.11/tkinter/__init__.py", line 1948, in __call__ return self.func(*args) ^^^^^^^^^^^^^^^^ File "/home/…/calculator.py", line 72, in on_leave e.widget['background'] = 'SystemButtonFace' ~~~~~~~~^^^^^^^^^^^^^^ File "/usr/lib/python3.11/tkinter/__init__.py", line 1713, in __setitem__ self.configure({key: value}) File "/usr/lib/python3.11/tkinter/__init__.py", line 1702, in configure return self._configure('configure', cnf, kw) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.11/tkinter/__init__.py", line 1692, in _configure self.tk.call(_flatten((self._w, cmd)) + self._options(cnf)) _tkinter.TclError: unknown color name "SystemButtonFace"
Yeah, not a great start… Chances are that, on anything except Windows, SystemButtonFace
is not a valid background color and you need to store it from a default button to be able to restore it later. One way would be to use yet another global variable to store it from your create_button
calls. A better alternative would be to create a closure that will remember this value and use it as your event handler. This will also allows you to consolidate on_enter
and on_leave
into a single function:
def change_background_handler(color): def on_event(event): event.widget['background'] = color return on_event def create_button(…): … button.bind("<Enter>", change_background_handler('#fafafa')) button.bind("<Leave>", change_background_handler(button['bg']))
Chaining operations
Using a calculator, it is expected that selecting an operator right after a computation is performed will reuse the result as the first operand. But:
- One should still be able to enter a completely new first operand after pressing the
=
sign, which your code forbid. - It is expected that selecting a second operator performs an implicit press on the
=
sign just before. Instead, your code wrongfully discard the second operator input.
The fix is rather simple as you just need to use 0
and "plus"
as your default first operand and mode (and reset to it after a press on =
), and have your do_operator_mode
call do_equal_mode
first. This will allows you to get rid of all the checks of the form mode == 'xxx_input'
.
In order to be able to reset the display if the user want to enter a fresh new first operand after a press on =
, you may need to introduce a new variable that tells you if you need to clear the display
or not.
Improvements
Operators
Instead of using strings to convey information about the operation to perform, have a look at the operator
module. You can directly pass operator.add
, operator.sub
, operator.mul
, operator.truediv
around instead.
Buttons
You already know about unpacking, let's leverage that to remove all your button_X
functions. Their main purpose is to transform a multi-parameter function like your do_xxx_mode
into a parameter-less function suitable for the button command
argument. But you can create them on the fly:
def create_button(root, text, row, col, *command): if not command: on_click = None else: command, *arguments = command def on_click(): command(*arguments) button = tk.Button(root, text=text, padx=20, pady=20, command=on_click) …
Window creation
Since you already manually list the parameters to all your create_button
calls, the for loop does not add much here and you would be better of just calling it with the appropriate parameters:
create_button(root, '0', 4, 0, add_character_to_display, '0') create_button(root, '1', 3, 0, add_character_to_display, '1') create_button(root, '2', 3, 1, add_character_to_display, '2') create_button(root, '3', 3, 2, add_character_to_display, '3') create_button(root, '4', 2, 0, add_character_to_display, '4') create_button(root, '5', 2, 1, add_character_to_display, '5') create_button(root, '6', 2, 2, add_character_to_display, '6') create_button(root, '7', 1, 0, add_character_to_display, '7') create_button(root, '8', 1, 1, add_character_to_display, '8') create_button(root, '9', 1, 2, add_character_to_display, '9') create_button(root, '.', 4, 1, add_character_to_display, '.') create_button(root, '+', 4, 3, do_operator_mode, operator.add) create_button(root, '-', 3, 3, do_operator_mode, operator.sub) create_button(root, '*', 2, 3, do_operator_mode, operator.mul) create_button(root, '/', 1, 3, do_operator_mode, operator.truediv) create_button(root, '=', 4, 2, do_equal_mode)
Globals
Using globals like this is not ideal, consider passing the relevant variables around as parameters to functions. Consider using a list (meh) or a class to store values that are tied with the same concept and reduce the number of arguments to your functions.
Top-level code
Instead of putting blocks of code at the top-level of your code, consider using an if __name__ == '__main__'
guard. It will make the code easier to deal with when importing / testing it.
Further
As with many GUI, it is advisable to regroup dependent parts of you application into a class to simplify reasoning about and ease passing of variables / context. To me, the whole Entry
+ Button
s you define here would be better of in a class like that.
Proposed improvements
import tkinter as tk from operator import add, sub, mul, truediv class Calculator: def __init__(self, root): display = tk.Entry(root, width=30) display.insert(tk.END, '0') display.grid(row=0, column=0, columnspan=4) self.add_button(root, '0', 4, 0, self.new_character, '0') self.add_button(root, '1', 3, 0, self.new_character, '1') self.add_button(root, '2', 3, 1, self.new_character, '2') self.add_button(root, '3', 3, 2, self.new_character, '3') self.add_button(root, '4', 2, 0, self.new_character, '4') self.add_button(root, '5', 2, 1, self.new_character, '5') self.add_button(root, '6', 2, 2, self.new_character, '6') self.add_button(root, '7', 1, 0, self.new_character, '7') self.add_button(root, '8', 1, 1, self.new_character, '8') self.add_button(root, '9', 1, 2, self.new_character, '9') self.add_button(root, '.', 4, 1, self.new_character, '.') self.add_button(root, '+', 4, 3, self.new_operation, add) self.add_button(root, '-', 3, 3, self.new_operation, sub) self.add_button(root, '*', 2, 3, self.new_operation, mul) self.add_button(root, '/', 1, 3, self.new_operation, truediv) self.add_button(root, '=', 4, 2, self.perform_operation) self.display = display self.operator = add self.previous_result = 0 self.need_display_refresh = False @staticmethod def add_button(root, text, row, col, *command): if not command: on_click = None else: command, *arguments = command def on_click(): command(*arguments) button = tk.Button(root, text=text, padx=20, pady=20, command=on_click) button.grid(row=row, column=col) button.bind("<Enter>", change_background_handler('#fafafa')) button.bind("<Leave>", change_background_handler(button['bg'])) def parse_display(self): content = self.display.get() if '.' in content: return float(content) return int(content) def new_character(self, new_number): current_text = self.display.get() if self.need_display_refresh or (new_number != '.' and only_zeroes(current_text)): self.display.delete(0, tk.END) self.display.insert(tk.END, new_number) self.need_display_refresh = False def new_operation(self, operator): self.perform_operation() self.previous_result = self.parse_display() self.operator = operator def perform_operation(self): result = self.operator(self.previous_result, self.parse_display()) self.display.delete(0, tk.END) self.display.insert(tk.END, str(result)) self.need_display_refresh = True self.operator = add self.previous_result = 0 def change_background_handler(color): def on_event(event): event.widget['background'] = color return on_event def only_zeroes(text): return all(char == '0' for char in text) if __name__ == '__main__': root = tk.Tk() Calculator(root) root.mainloop()
TODOs
At least two buttons feels missing from your calculator: "clear" and "remove the last character".
You'll also need to improve the way numbers are retrieved from the Entry
and properly handle inputs such as 3.14.159...
which will otherwise leave your calculator in a broken state.