3
\$\begingroup\$

I've written a simple unconventional calculator application using Python and tkinter. It's unconventional because you use the "modes" (+ - * /) to add to the current sum, instead of entering in a statement and doing that calculation. Because of this, 0 is not included. The main part of my program I would appreciate being reviewed:

Creating The Number Buttons

I absolutely hate large blocks of code that are essentially the same except for one or two things. In this case, it's creating the buttons. I am certain it is possible doing it with a loop, but everything I've attempted has resulted in failure. Everything works as intended, but I'd really appreciate some suggestions pertaining to the number buttons creation. As always, other feedback is welcome and greatly appreciated.

from tkinter import Tk, Label, Button

class Calculator:

    def __init__(self, master: Tk):
        # Initial Setup #
        self.master = master
        self.master.title("Calculator")
        self.master.maxsize(500, 700)

        self.sum = 0
        self.mode = "+"

        # Number Buttons
        self.number_buttons = [
            Button(master, text="1", command=lambda: self.do_math(1)),
            Button(master, text="2", command=lambda: self.do_math(2)),
            Button(master, text="3", command=lambda: self.do_math(3)),
            Button(master, text="4", command=lambda: self.do_math(4)),
            Button(master, text="5", command=lambda: self.do_math(5)),
            Button(master, text="6", command=lambda: self.do_math(6)),
            Button(master, text="7", command=lambda: self.do_math(7)),
            Button(master, text="8", command=lambda: self.do_math(8)),
            Button(master, text="9", command=lambda: self.do_math(9))
        ]

        # Mode Button and Label
        self.mode_label = Label(master, text=f"Mode: {self.mode}")
        self.mode_button = Button(master, text="Change Mode", command=self.change_mode)

        # Sum Label Setup #
        self.sum_label = Label(master, text=str(self.sum))
        self.sum_label.grid(row=1, column=2)

        # Reset Button Setup #
        self.reset_button = Button(master, text="Reset", command=self.reset_sum)

        # Calculate/Set Layout for Buttons #
        r, c = 2, 1
        for number_button in self.number_buttons:
            number_button.configure(width=10, height=10)
            number_button.grid(row=r, column=c)
            if c % 3 == 0:
                r += 1
                c = 0
            c += 1

        # Add Mode Label/Button #
        self.mode_label.grid(row=5, column=2)
        self.mode_button.grid(row=6, column=2)

        # Add Reset Button #
        self.reset_button.grid(row=7, column=2)

    def do_math(self, num: int) -> None:
        """
        Checks the current mode and modifies the sum

        :param num -> int: Button clicked by the user

        :return: None

        """
        modes = {
            "+": self.sum + num,
            "-": self.sum - num,
            "*": self.sum * num,
            "/": self.sum / num
        }
        self.sum = modes[self.mode]
        self.sum_label.configure(text=str(self.sum))

    def change_mode(self) -> None:
        """
        Changes the mode in (+, -, *, /) rotation

        :return: None

        """
        modes = {
            "+": "-",
            "-": "*",
            "*": "/",
            "/": "+"
        }
        self.mode = modes[self.mode]
        self.mode_label.configure(text=f"Mode: {self.mode}")

    def reset_sum(self) -> None:
        """
        Resets the sum

        :return: None

        """
        self.sum = 0
        self.sum_label.configure(text=str(self.sum))

if __name__ == "__main__":
    root = Tk()
    gui = Calculator(root)
    root.resizable(width=False, height=False)
    root.mainloop()
\$\endgroup\$
  • \$\begingroup\$ If I understood the basic idea correctly, this is not as "unusual" as you might think. Older calculators and also the software calculator on Windows work like this. However, your comment about not needing 0 threw me off a little bit. How would one express something like 1000+5000 in your calculator? \$\endgroup\$ – AlexV Nov 5 '19 at 12:10
5
\$\begingroup\$

To reduce duplication, look at what's identical in each line, and what's different. For the different parts, make them a part of a loop, or the arguments to a function. For the identical parts, make them the body of the function/loop. In this case, look at the lines:

Button(master, text="1", command=lambda: self.do_math(1)),
Button(master, text="2", command=lambda: self.do_math(2)),
Button(master, text="3", command=lambda: self.do_math(3)),

The only thing that differs are the text parameter values, and the argument to do_math. Conveniently, they're the same, so this can be a simple loop over a range of numbers. I'm going to use a list comprehension here:

self.number_buttons = [Button(master, text=str(n), command=lambda n: self.do_math(n))
                       for n in range(1, 10)]

And in most cases, that would be fine. Unfortunately though, you're needing to put n inside of a lambda, and that can cause some surprising problems. To fix it, I'm going to use the lambda n=n: . . . fix. If I didn't make this change, the text would be set fine, but each button would end up being passed 19 instead of the correct number.

self.number_buttons = [Button(master, text=str(n), command=lambda n=n: self.do_math(n))
                       for n in range(1, 10)]

modes also has some duplication that can be improved.

modes = {
    "+": self.sum + num,
    "-": self.sum - num,
    "*": self.sum * num,
    "/": self.sum / num
}

self.sum = modes[self.mode]

You're repeating self.sum and num over and over. You can just deal with the data later by mapping to functions instead of the sum. The operator module has functions that correspond to the common operators like + to make this easy:

import operator as op

mode_ops = {
    "+": op.add,
    "-": op.sub,
    "*": op.mul,
    "/": op.truediv
}

f = mode_ops[self.mode]
self.sum = f(self.sum, num)  # Now we only need to specify the data once

This also saves you from computing every possible answer ahead of time. If you ever added an expensive operator, this could save some overhead (although it would be unlikely that that would ever be a major problem).

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ I knew there was a way with a loop! I used your exact loop but the n=n. It was driving me crazy, thank you! \$\endgroup\$ – Linny Nov 5 '19 at 3:13

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.