HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Python Calculator using tkinter

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
usingcalculatorpythontkinter

Problem

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,

Solution

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, colu

Code Snippets

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.
 

Context

StackExchange Code Review Q#141633, answer score: 3

Revisions (0)

No revisions yet.