Python Calculator using tkinter

Question

I created a basic desktop calculator using Python. Does anyone have any suggestions as far as coding best practices, readability issues, or just generic coding mistakes I might have missed? Any professional input you could provide would be greatly appreciated.

import tkinter as Tk


class Calculator:
    ''' main class that constructs the calc and preforms the calculations '''

    def __init__(self, master):
        # Global variables needed throughout a single calculation
        self.top_string = ''    # string for label at top of calculator
        self.number1 = 0       # storage of first number selected
        self.number2 = 0       # storage of second number selected
        self.add = False       # + boolean
        self.subtract = False  # - boolean
        self.multiply = False  # x boolean
        self.divide = False  # / boolean

        # Top Label Layout
        self.label = Tk.Label(master, text='0', bg='black', fg='white', height=2, width=4)
        self.label.grid(row=0, column=0, columnspan=4, sticky=Tk.N+Tk.E+Tk.S+Tk.W)
        self.label.config(font='Verdana 16 bold')

        # Button Layout
        Tk.Button(master, text='1', height=2, width=6, command=lambda: self.number_pressed(1)).grid(row=1, column=0)
        Tk.Button(master, text='2', height=2, width=6, command=lambda: self.number_pressed(2)).grid(row=1, column=1)
        Tk.Button(master, text='3', height=2, width=6, command=lambda: self.number_pressed(3)).grid(row=1, column=2)
        Tk.Button(master, text='4', height=2, width=6, command=lambda: self.number_pressed(4)).grid(row=2, column=0)
        Tk.Button(master, text='5', height=2, width=6, command=lambda: self.number_pressed(5)).grid(row=2, column=1)
        Tk.Button(master, text='6', height=2, width=6, command=lambda: self.number_pressed(6)).grid(row=2, column=2)
        Tk.Button(master, text='7', height=2, width=6, command=lambda: self.number_pressed(7)).grid(row=3, column=0)
        Tk.Button(master, text='8', height=2, width=6, command=lambda: self.number_pressed(8)).grid(row=3, column=1)
        Tk.Button(master, text='9', height=2, width=6, command=lambda: self.number_pressed(9)).grid(row=3, column=2)
        Tk.Button(master, text='0', command=lambda: self.number_pressed(0)).grid(row=4, columnspan=2, sticky=Tk.N+Tk.E+Tk.S+Tk.W)
        Tk.Button(master, text='+', height=2, width=6, command=lambda: self.sign_pressed("+")).grid(row=1, column=3)
        Tk.Button(master, text='-', height=2, width=6, command=lambda: self.sign_pressed("-")).grid(row=2, column=3)
        Tk.Button(master, text='x', height=2, width=6, command=lambda: self.sign_pressed("*")).grid(row=3, column=3)
        Tk.Button(master, text='/', height=2, width=6, command=lambda: self.sign_pressed("/")).grid(row=4, column=3)
        Tk.Button(master, text='C', height=2, width=6, command=self.clear_all).grid(row=4, column=2)
        Tk.Button(master, text='=', height=2, command=self.equals).grid(row=5, columnspan=4, sticky=Tk.N+Tk.E+Tk.S+Tk.W)

    def number_pressed(self, button_number):
        ''' This function is triggered when buttons 0 - 9 are pushed '''
        if self.number1 is 0 and not any([self.add, self.subtract, self.multiply, self.divide]):
            self.number1 = button_number
            self.top_string = str(button_number)
            self.label.config(text=str(button_number))

        elif self.number1 is not 0 and not any([self.add, self.subtract, self.multiply, self.divide]):
            self.top_string += str(button_number)
            self.number1 = int(self.top_string)
            self.label.config(text=self.top_string)

        elif self.number2 is 0:
            self.number2 = button_number
            self.top_string = str(button_number)
            self.label.config(text=str(button_number))

        elif self.number1 is not 0:
            self.top_string += str(button_number)
            self.number2 = int(self.top_string)
            self.label.config(text=self.top_string)


    def sign_pressed(self, sign):
        ''' This function is triggered when +,-,*, or / is pushed. First check num1 and num2 are already storage.
        If so, it performs an num1 equals total, then displays num1, then resets the sign to the last on pushed.
        Which allows for multiply calculations before pushing = button '''
        if self.number2 is not 0 and self.number1 is not 0:
            self.number1 = self.equals()
            self.label.config(text=str(self.number1))
            self.top_string = ''
        if sign is "+":
            self.add = True
            self.subtract = False  # - boolean
            self.multiply = False  # x boolean
            self.divide = False
        if sign is "-":
            self.add = False
            self.subtract = True  # - boolean
            self.multiply = False  # x boolean
            self.divide = False
        if sign is "*":
            self.add = False
            self.subtract = False  # - boolean
            self.multiply = True  # x boolean
            self.divide = False
        if sign is "/":
            self.add = False
            self.subtract = False  # - boolean
            self.multiply = False  # x boolean
            self.divide = True
        else:
            if sign is "+":
                self.add = True
            if sign is "-":
                self.subtract = True
            if sign is "*":
                self.multiply = True
            if sign is "/":
                self.divide = True

    def equals(self):
        ''' Triggers calculation then clears all vars '''
        total = 0
        if self.add is True:
            total = self.number1 + self.number2
            self.number2 = 0    # resets for next calculation if clear is not presses

        elif self.subtract is True:
            total = self.number1 - self.number2
            self.number2 = 0

        elif self.multiply is True:
            total = self.number1 * self.number2
            self.number2 = 0

        elif self.divide is True:
            total = round(self.number1 / self.number2, 3)
            total = int(total) if total.is_integer() else total
            self.number2 = 0

        self.top_string = ''
        self.add = False
        self.subtract = False
        self.multiply = False
        self.divide = False
        self.label.config(text=str(total))
        return total

    def clear_all(self):
        '''Clears all vars'''
        self.top_string = ''    # first string to appear after selecting sign ( 9 + )
        self.number1 = 0       # storage of first number selected
        self.number2 = 0       # storage of second number selected
        self.add = False       # + boolean
        self.subtract = False  # - boolean
        self.multiply = False  # x boolean
        self.divide = False    # / boolean
        self.label.config(text='0')  # top label

if __name__ == '__main__':
    ROOT = Tk.Tk()
    ROOT.wm_title('Calculator')
    ROOT.resizable(width=False, height=False)
    Calculator(ROOT)
    ROOT.mainloop()

Answer 1

Code Quality

Simplifying code

• Try adding a main function or static method to your program, this way anyone who imports your class or file can just run the method if they want your code to execute as if it had been run manually.

• Lines 23 through 38, the block of Tk.Button calls, can be simplified with a for loop. I've attempted to do this in the reformatted code.

• The top_string variable is unnecessary, you can access and change the text of the label directly by treating it as a dictionary, e.g. self.label['text'].

• In the calculate_result method, you should note that since all the self.*operator* variables are Boolean you can just check their values directly without comparing them to True or False.

• Another thing to note is that if you're going to be resetting the self.second_number_selected variable to 0 in all your if statements, it'd be easier to set up the logic so that you can move that code outside of the conditional blocks and just have one line of code that resets things every time regardless.

Coding Standards

• Look into using PEP8 to standardize the style in your code. There are several online PEP8 checkers that you can copy-paste your code into to check if it's following all of the conventions.

• Consider renaming your variables to more clearly express their purpose. For example, your comments provide excellent alternative names for the variables number1 and number2, those alternative names being first_number_selected and second_number_selected as these more clearly define what is stored in these variables.

• The self.label variable would be better named something like self.result_label or self.display_label. It should indicate its purpose more clearly. (In the modified code, self.label has been renamed to self.display_label since it is the label that displays whatever the relevant text is depending on what the user has pressed.)

---

Code Problems

• You are misusing the is operator as you are using it to check the value of the variables, which is not its purpose. The is operator compares the memory addresses of the objects to check if they're the same object not to check if what is contained in the object has the same value. (This is the default behavior of the == operator when comparing objects in Java, if you've ever used it.) For these cases, just saying value == some_number is all that is required to compare values.

---

Suggestions/Ideas

• Perhaps it would be better to not clear all your variables after a result is complete and to only clear some of them, setting up the calculator to use the result as the first number in future calculations (As you'll notice a lot of handheld calculators do). It seems you have the groundwork for this in your code as you have the line self.second_number_selected = 0 in all cases, but you're missing the self.first_number_selected = result line that would do this for you.

• I got the idea that combining the operators and whether they've been selected into a dictionary makes the resetting process easier, greatly simplifies the sign_pressed method, and allows you to expand the functionality a bit more easily in the future.

There are more things that I could list, but try taking a look through the code I've modified so far and see what you can find out.

import tkinter as Tk
import math


class Calculator:
    """Main class that constructs the calculator and handles user input and calculations"""

    def __init__(self, parent):
        # Global variables needed throughout a single calculation
        self.operations = {'+': False,
                           '-': False,
                           '*': False,
                           '/': False}
        self.first_number_selected = 0
        self.second_number_selected = 0

        # Expands a button to fill all the open grid-space around it.
        expand_button = Tk.N + Tk.E + Tk.S + Tk.W

        # Display label setup
        self.display_label = Tk.Label(parent, text='0', font='Verdana 16 bold',
                                      bg='black', fg='white', height=2, width=4)
        self.display_label.grid(row=0, column=0, columnspan=4, sticky=expand_button)

        # Button Layout
        # Number buttons
        Tk.Button(parent, text='0', command=self._number_callback(0)).grid(row=4, columnspan=2, sticky=expand_button)
        for number in range(1, 10):
            callback = self._number_callback(number)
            row = math.ceil(number/3)
            col = (number-1) % 3
            Tk.Button(parent, text=str(number), height=2, width=6, command=callback).grid(row=row, column=col)
        # Operation buttons
        for position, operator in enumerate(self.operations.keys()):
            Tk.Button(parent, text=operator, height=2, width=6,
                      command=self._operation_callback(operator)).grid(row=position + 1, column=3)
        Tk.Button(parent, text='C', height=2, width=6, command=self.reset).grid(row=4, column=2)
        Tk.Button(parent, text='=', height=2, command=self.calculate_result).grid(row=5, columnspan=4, sticky=expand_button)

    def number_pressed(self, button_number):
        """This function is called when buttons 0 - 9 are pushed"""

        if not any(self.operations.values()):
            if self.first_number_selected == 0:
                self.first_number_selected = button_number
                self.display_label['text'] = str(button_number)
            else:
                self.display_label['text'] += str(button_number)
                self.first_number_selected = int(self.display_label['text'])
        elif self.second_number_selected == 0:
            self.second_number_selected = button_number
            self.display_label['text'] = str(button_number)
        else:
            self.display_label['text'] += str(button_number)
            self.second_number_selected = int(self.display_label['text'])

    def _number_callback(self, number):
        """Helper method that is creates a callback function
        for each of the buttons that indicate numbers."""
        return lambda: self.number_pressed(number)

    def operation_selected(self, operation):
        """This function is triggered when +,-,*, or / is pushed.
        First check if the first and second numbers are already selected.
        If so, it calculates the result of the operation and displays it,
            then it sets the operation to the last one pushed.
        This allows for multiply calculations before the '=' button is pushed."""
        if self.second_number_selected and self.first_number_selected:
            self.first_number_selected = self.calculate_result()
            self.display_label['text'] = str(self.first_number_selected)

        self._reset_operations()
        self.operations[operation] = True

    def _operation_callback(self, sign):
        """Helper method that is creates a callback function
        for each of the buttons that indicate operations."""
        return lambda: self.operation_selected(sign)

    def _reset_operations(self):
        """Reset the press-status of all the operations"""
        for operator in self.operations.keys():
            self.operations[operator] = False

    def calculate_result(self):
        """Performs calculation then sets up variables for future operations with the resulting number"""
        result = self.first_number_selected

        # The code can probably be rewritten in such a way that the operation that is to be performed
        # is directly accessible from the `operations` dictionary. This way if you add more operations
        # in the future, you wouldn't have to rewrite these methods in order to have them function
        # properly.
        if self.operations['+']:
            result = self.first_number_selected + self.second_number_selected
        elif self.operations['-']:
            result = self.first_number_selected - self.second_number_selected
        elif self.operations['*']:
            result = self.first_number_selected * self.second_number_selected
        elif self.operations['/']:
            result = round(self.first_number_selected / self.second_number_selected, 3)
            result = int(result) if result.is_integer() else result

        self.first_number_selected = result
        self.second_number_selected = 0  # Resets for next calculation if the clear button is not pressed
        self._reset_operations()
        self.display_label['text'] = str(result)
        return result

    def reset(self):
        """Reset all the variables to their default states"""
        self.first_number_selected = 0
        self.second_number_selected = 0
        self._reset_operations()
        self.display_label['text'] = '0'

    @staticmethod
    def run():
        frame = Tk.Tk()
        frame.wm_title('Calculator')
        frame.resizable(width=False, height=False)
        Calculator(frame)
        frame.mainloop()


def main():
    Calculator.run()

if __name__ == '__main__':
    main()

The modifications I made were to both shorten the code and to allow it to be a bit more flexible in the future if you ever wanted to add more operators. I added a comment in the calculate_result function detailing what you'd have to change in the code in the event you wanted to try this.