2
\$\begingroup\$

So this is my first project.

I made a Calculator using Tkinter.

For the next version, I will try adding

  • oops concepts
  • Custom parser for input

Here's the code

#!/usr/bin/env python3.4
from tkinter import *
import parser

root = Tk()
root.title('Calculator')

i = 0

def factorial():
    """Calculates the factorial of the number entered."""
    whole_string = display.get()
    number = int(whole_string)
    fact = 1
    counter = number 
    try:
        while counter > 0:
            fact = fact*counter
            counter -= 1
        clear_all()
        display.insert(0, fact)
    except Exception:
        clear_all()
        display.insert(0, "Error")


def clear_all():
    """clears all the content in the Entry widget"""
    display.delete(0, END)

def get_variables(num):
    """Gets the user input for operands and puts it inside the entry widget"""
    global i
    display.insert(i, num)
    i += 1

def get_operation(operator):
    """Gets the operand the user wants to apply on the functions"""
    global i
    length = len(operator)
    display.insert(i, operator)
    i += length

def undo():
    """removes the last entered operator/variable from entry widget"""
    whole_string = display.get()
    if len(whole_string):        ## repeats until
        ## now just decrement the string by one index
        new_string = whole_string[:-1]
        print(new_string)
        clear_all()
        display.insert(0, new_string)
    else:
        clear_all() 
        display.insert(0, "Error, press AC")

def calculate():
    """
    Evaluates the expression
    ref : http://stackoverflow.com/questions/594266/equation-parsing-in-python
    """
    whole_string = display.get()
    try:
        formulae = parser.expr(whole_string).compile()
        result = eval(formulae)
        clear_all()
        display.insert(0, result)
    except Exception:
        clear_all()
        display.insert(0, "Error!")

root.columnconfigure(0,pad=3)
root.columnconfigure(1,pad=3)
root.columnconfigure(2,pad=3)
root.columnconfigure(3,pad=3)
root.columnconfigure(4,pad=3)

root.rowconfigure(0,pad=3)
root.rowconfigure(1,pad=3)
root.rowconfigure(2,pad=3)
root.rowconfigure(3,pad=3)

display = Entry(root, font = ("Calibri", 13))
display.grid(row = 1, columnspan = 6    , sticky = W+E)

one = Button(root, text = "1", command = lambda : get_variables(1), font=("Calibri", 12))
one.grid(row = 2, column = 0)
two = Button(root, text = "2", command = lambda : get_variables(2), font=("Calibri", 12))
two.grid(row = 2, column = 1)
three = Button(root, text = "3", command = lambda : get_variables(3), font=("Calibri", 12))
three.grid(row = 2, column = 2)

four = Button(root, text = "4", command = lambda : get_variables(4), font=("Calibri", 12))
four.grid(row = 3 , column = 0)
five = Button(root, text = "5", command = lambda : get_variables(5), font=("Calibri", 12))
five.grid(row = 3, column = 1)
six = Button(root, text = "6", command = lambda : get_variables(6), font=("Calibri", 12))
six.grid(row = 3, column = 2)

seven = Button(root, text = "7", command = lambda : get_variables(7), font=("Calibri", 12))
seven.grid(row = 4, column = 0)
eight = Button(root, text = "8", command = lambda : get_variables(8), font=("Calibri", 12))
eight.grid(row = 4, column = 1)
nine = Button(root , text = "9", command = lambda : get_variables(9), font=("Calibri", 12))
nine.grid(row = 4, column = 2)

cls = Button(root, text = "AC", command = clear_all, font=("Calibri", 12), foreground = "red")
cls.grid(row = 5, column = 0)
zero = Button(root, text = "0", command = lambda : get_variables(0), font=("Calibri", 12))
zero.grid(row = 5, column = 1)
result = Button(root, text = "=", command = calculate, font=("Calibri", 12), foreground = "red")
result.grid(row = 5, column = 2)

plus = Button(root, text = "+", command =  lambda : get_operation("+"), font=("Calibri", 12))
plus.grid(row = 2, column = 3)
minus = Button(root, text = "-", command =  lambda : get_operation("-"), font=("Calibri", 12))
minus.grid(row = 3, column = 3)
multiply = Button(root,text = "*", command =  lambda : get_operation("*"), font=("Calibri", 12))
multiply.grid(row = 4, column = 3)
divide = Button(root, text = "/", command = lambda :  get_operation("/"), font=("Calibri", 12))
divide.grid(row = 5, column = 3)

# adding new operations
pi = Button(root, text = "pi", command = lambda: get_operation("*3.14"), font =("Calibri", 12))
pi.grid(row = 2, column = 4)
modulo = Button(root, text = "%", command = lambda :  get_operation("%"), font=("Calibri", 12))
modulo.grid(row = 3, column = 4)
left_bracket = Button(root, text = "(", command = lambda: get_operation("("), font =("Calibri", 12))
left_bracket.grid(row = 4, column = 4)
exp = Button(root, text = "exp", command = lambda: get_operation("**"), font = ("Calibri", 10))
exp.grid(row = 5, column = 4)

## To be added :
# sin, cos, log, ln
undo_button = Button(root, text = "<-", command = undo, font =("Calibri", 12), foreground = "red")
undo_button.grid(row = 2, column = 5)
fact = Button(root, text = "x!", command = factorial, font=("Calibri", 12))
fact.grid(row = 3, column = 5)
right_bracket = Button(root, text = ")", command = lambda: get_operation(")"), font =("Calibri", 12))
right_bracket.grid(row = 4, column = 5)
square = Button(root, text = "^2", command = lambda: get_operation("**2"), font = ("Calibri", 10))
square.grid(row = 5, column = 5)

root.mainloop()

Any Suggestions on how could I improve upon it guys?

Edit:

Here's the link to the repo if anybody wants to download the executable for this.

https://github.com/prodicus/pyCalc

\$\endgroup\$
3
\$\begingroup\$

Use an object oriented structure

You mention this in your question. It definitely makes it easier to organize your code. I recommend reading this question:

https://stackoverflow.com/q/17466561/7432

Don't use global imports

PEP8 discourages global imports. Experience has shown that doing so leads to code that can be hard to maintain over time.

Change this:

from tkinter import *

to this:

import tkinter as tk

This will require you to prefix tk. to all of the tk classes. This is a Good Thing. The Zen of Python tells us that explicit is better than implicit. For example:

root = tk.Tk()
...
display = tk.Entry(root, ...)

Use a named font

One of the really great features of Tkinter is the notion of "named fonts". Create a custom font, and use that for your widgets rather than hard-coding the font in each widget. If you decide to change the font later, you only have to change one line of code.

As a plus, if you change the font at runtime (eg: give the user an "increase font / decrease font" menu item), all of the widgets that use this font will automatically and instantly change.

import tkinter.font
customFont = tkinter.font.Font(family=font="Calibri", size=12)
one = Button(..., font=customFont)

Create your buttons in a loop

You are creating a bunch of numeric buttons that are nearly identical. I suggest creating them in a loop to cut down on the number of lines. For example:

buttons =[]
for i in range(0,10):
    button = tk.Button(root, text=str(i), font=customFont,
                       command=lambda num=i: get_variables(num))
    buttons.append(button)

Separate layout from widget creation

It is much easier to visualize the layout of your widgets if you separate the creation of the widgets from the layout of the widgets. Assuming you are creating your widgets in a loop, you can then lay them out easily in one clear block of code:

buttons[1].grid(row=2, column=0)
buttons[2].grid(row=2, column=1)
buttons[3].grid(row=2, column=2)
buttons[4].grid(row=3, column=0)
buttons[5].grid(row=3, column=1)
buttons[6].grid(row=3, column=2)
buttons[7].grid(row=4, column=0)
buttons[8].grid(row=4, column=1)
buttons[9].grid(row=4, column=2)

With this simple example it's not overly important, but this is a good habit to get into. This becomes more true when you have a complex layout with widgets that span various rows and columns.

| improve this answer | |
\$\endgroup\$
2
\$\begingroup\$

Weird factorial

From a user-interface point of view, the factorial is very weird. All the others buttons just add the symbol to the display, but factorial actually evaluates the whole expression. The principle of last suprise states that if 9 buttons do a thing, the 10-th should either:

  1. Behave similarly to them
  2. Have a special mark on it to signify speciality. (On a side note making the AC, <- and = red really improves user experience in my opinion.)

Constants

If you use the same value many times, you should give it a name, so that modifying it is faster.

You use ("Calibri", 12) 22 times in your code! What a boring day if you decide to make the font bigger!

Instead use:

FONT = ("Calibri", 12)

And then replace ("Calibri", 12) with FONT to allow for very fast code upgrades.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks for that. Would you shed some light on what are the advantages of adding class'es to a GUI programs in general? \$\endgroup\$ – Tasdik Rahman Nov 6 '15 at 17:43
  • \$\begingroup\$ @prodicus I am not expert in OO, but usually waiting some days gets good in depth answers, at first each question receives only appetizers. \$\endgroup\$ – Caridorc Nov 6 '15 at 17:45
  • \$\begingroup\$ No problem. Do you know how to make an executable for windows machine out of this code if I am on Ubuntu? \$\endgroup\$ – Tasdik Rahman Nov 6 '15 at 17:56
  • \$\begingroup\$ @prodicus please be careful with on-topic-ness. We review the code by saying what we know, not answer any question vaguely regarding the topic. \$\endgroup\$ – Caridorc Nov 6 '15 at 17:59
  • 1
    \$\begingroup\$ Sorry for going off topic. Thanks for your inputs by the way. \$\endgroup\$ – Tasdik Rahman Nov 7 '15 at 0:38
1
\$\begingroup\$

Instead of repeating code a bunch:

root.columnconfigure(0,pad=3)
root.columnconfigure(1,pad=3)
root.columnconfigure(2,pad=3)
root.columnconfigure(3,pad=3)
root.columnconfigure(4,pad=3)

root.rowconfigure(0,pad=3)
root.rowconfigure(1,pad=3)
root.rowconfigure(2,pad=3)
root.rowconfigure(3,pad=3)

Surely you can do two loops:

for i in range(4):
    root.columnconfigure(i, pad=3)

for i in range(3):
    root.rowconfigure(i, pad=3)

It'd be even better if 4 and 3 were named constants that explained the seeming arbitrary values. Same with pad too.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks for that. I noticed now, that I have an inconsistency in the interface. How should I refactor the factorial part? \$\endgroup\$ – Tasdik Rahman Nov 6 '15 at 17:40

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.