Python 3 calculator with tkinter

Question

I have been programming for a long time. Only recently have I decided to take a stab at Python (I should be working with C# as I am in school for it, but I don't care for Windows, long story).

I was on this site and it showed a source for a calculator. I took it, and put it in PyCharm and started to study. By the time I was done I had changed the source significantly. I had added keyboard binding and reduced a lot of the redundant code in it.

My question is simple: is this code that I wrote efficient from a python standard viewpoint?

# -*-coding: utf-8-*-
# !/usr/bin/python3.5
from tkinter import Tk, Button, Entry, END
import math
class Calc:
 def getandreplace(self): # replace x, + and % to symbols that can be used in calculations
 # we wont re write this to the text box until we are done with calculations
 self.txt = self.e.get() # Get value from text box and assign it to the global txt var
 self.txt = self.txt.replace('÷', '/')
 self.txt = self.txt.replace('x', '*')
 self.txt = self.txt.replace('%', '/100')
 def evaluation(self, specfunc): # Evaluate the items in the text box for calculation specfunc = eq, sqroot or power
 self.getandreplace()
 try:
 self.txt = eval(str(self.txt)) # evaluate the expression using the eval function
 except SyntaxError:
 self.displayinvalid()
 else:
 if any([specfunc == 'sqroot', specfunc == 'power']): # Square Root and Power are special
 self.txt = self.evalspecialfunctions(specfunc)
 self.refreshtext()
 def displayinvalid(self):
 self.e.delete(0, END)
 self.e.insert(0, 'Invalid Input!')
 def refreshtext(self): # Delete current contents of textbox and replace with our completed evaluatioin
 self.e.delete(0, END)
 self.e.insert(0, self.txt)
 def evalspecialfunctions(self, specfunc): # Calculate square root and power if specfunc is sqroot or power
 if specfunc == 'sqroot':
 return math.sqrt(float(self.txt))
 elif specfunc == 'power':
 return math.pow(float(self.txt), 2)
 def clearall(self): # AC button pressed on form or 'esc" pressed on keyboard
 self.e.delete(0, END)
 self.e.insert(0, '0')
 def clear1(self, event=None):
 # C button press on form or backspace press on keyboard event defined on keyboard press
 if event is None:
 self.txt = self.e.get()[:-1] # Form backspace done by hand
 else:
 self.txt = self.getvalue() # No need to manually delete when done from keyboard
 self.refreshtext()
 def action(self, argi: object): # Number or operator button pressed on form and passed in as argi
 self.txt = self.getvalue()
 self.stripfirstchar()
 self.e.insert(END, argi)
 def keyaction(self, event=None): # Key pressed on keyboard which defines event
 self.txt = self.getvalue()
 if any([event.char.isdigit(), event.char in '/*-+%().']):
 self.stripfirstchar()
 elif event.char == '\x08':
 self.clear1(event)
 elif event.char == '\x1b':
 self.clearall()
 elif event.char == '\r':
 self.evaluation('eq')
 else:
 self.displayinvalid()
 return 'break'
 def stripfirstchar(self): # Strips leading 0 from text box with first key or button is pressed
 if self.txt[0] == '0':
 self.e.delete(0, 1)
 def getvalue(self): # Returns value of the text box
 return self.e.get()
 def __init__(self, master): # Constructor method
 self.txt = 'o' # Global var to work with text box contents
 master.title('Calulator')
 master.geometry()
 self.e = Entry(master)
 self.e.grid(row=0, column=0, columnspan=6, pady=3)
 self.e.insert(0, '0')
 self.e.focus_set() # Sets focus on the text box text area
 # Generating Buttons
 Button(master, text="=", width=10, command=lambda: self.evaluation('eq')).grid(row=4, column=4, columnspan=2)
 Button(master, text='AC', width=3, command=lambda: self.clearall()).grid(row=1, column=4)
 Button(master, text='C', width=3, command=lambda: self.clear1()).grid(row=1, column=5)
 Button(master, text="+", width=3, command=lambda: self.action('+')).grid(row=4, column=3)
 Button(master, text="x", width=3, command=lambda: self.action('x')).grid(row=2, column=3)
 Button(master, text="-", width=3, command=lambda: self.action('-')).grid(row=3, column=3)
 Button(master, text="÷", width=3, command=lambda: self.action('÷')).grid(row=1, column=3)
 Button(master, text="%", width=3, command=lambda: self.action('%')).grid(row=4, column=2)
 Button(master, text="7", width=3, command=lambda: self.action('7')).grid(row=1, column=0)
 Button(master, text="8", width=3, command=lambda: self.action('8')).grid(row=1, column=1)
 Button(master, text="9", width=3, command=lambda: self.action('9')).grid(row=1, column=2)
 Button(master, text="4", width=3, command=lambda: self.action('4')).grid(row=2, column=0)
 Button(master, text="5", width=3, command=lambda: self.action('5')).grid(row=2, column=1)
 Button(master, text="6", width=3, command=lambda: self.action('6')).grid(row=2, column=2)
 Button(master, text="1", width=3, command=lambda: self.action('1')).grid(row=3, column=0)
 Button(master, text="2", width=3, command=lambda: self.action('2')).grid(row=3, column=1)
 Button(master, text="3", width=3, command=lambda: self.action('3')).grid(row=3, column=2)
 Button(master, text="0", width=3, command=lambda: self.action('0')).grid(row=4, column=0)
 Button(master, text=".", width=3, command=lambda: self.action('.')).grid(row=4, column=1)
 Button(master, text="(", width=3, command=lambda: self.action('(')).grid(row=2, column=4)
 Button(master, text=")", width=3, command=lambda: self.action(')')).grid(row=2, column=5)
 Button(master, text="√", width=3, command=lambda: self.evaluation('sqroot')).grid(row=3, column=4)
 Button(master, text="x2", width=3, command=lambda: self.evaluation('power')).grid(row=3, column=5)
 # bind key strokes
 self.e.bind('<Key>', lambda evt: self.keyaction(evt))
# Main
root = Tk()
obj = Calc(root) # object instantiated
root.mainloop()

I don't really care for the names of some of the function names and variable names. I like using descriptive names, so names like self.e would have been called self.textbox or something. These things are leftovers from the web copy I found and haven't changed them.

Per request, the original code for this is below.

#-*-coding: utf-8-*-
from Tkinter import *
import math
class calc:
 def getandreplace(self):
 """replace x with * and ÷ with /"""
 self.expression = self.e.get()
 self.newtext=self.expression.replace(self.newdiv,'/')
 self.newtext=self.newtext.replace('x','*')
 def equals(self):
 """when the equal button is pressed"""
 self.getandreplace()
 try: 
 self.value= eval(self.newtext) #evaluate the expression using the eval function
 except SyntaxError or NameErrror:
 self.e.delete(0,END)
 self.e.insert(0,'Invalid Input!')
 else:
 self.e.delete(0,END)
 self.e.insert(0,self.value)
 def squareroot(self):
 """squareroot method"""
 self.getandreplace()
 try: 
 self.value= eval(self.newtext) #evaluate the expression using the eval function
 except SyntaxError or NameErrror:
 self.e.delete(0,END)
 self.e.insert(0,'Invalid Input!')
 else:
 self.sqrtval=math.sqrt(self.value)
 self.e.delete(0,END)
 self.e.insert(0,self.sqrtval)
 def square(self):
 """square method"""
 self.getandreplace()
 try: 
 self.value= eval(self.newtext) #evaluate the expression using the eval function
 except SyntaxError or NameErrror:
 self.e.delete(0,END)
 self.e.insert(0,'Invalid Input!')
 else:
 self.sqval=math.pow(self.value,2)
 self.e.delete(0,END)
 self.e.insert(0,self.sqval)
 def clearall(self): 
 """when clear button is pressed,clears the text input area"""
 self.e.delete(0,END)
 def clear1(self):
 self.txt=self.e.get()[:-1]
 self.e.delete(0,END)
 self.e.insert(0,self.txt)
 def action(self,argi): 
 """pressed button's value is inserted into the end of the text area"""
 self.e.insert(END,argi)
 def __init__(self,master):
 """Constructor method"""
 master.title('Calulator') 
 master.geometry()
 self.e = Entry(master)
 self.e.grid(row=0,column=0,columnspan=6,pady=3)
 self.e.focus_set() #Sets focus on the input text area
 self.div='÷'
 self.newdiv=self.div.decode('utf-8')
 #Generating Buttons
 Button(master,text="=",width=10,command=lambda:self.equals()).grid(row=4, column=4,columnspan=2)
 Button(master,text='AC',width=3,command=lambda:self.clearall()).grid(row=1, column=4)
 Button(master,text='C',width=3,command=lambda:self.clear1()).grid(row=1, column=5)
 Button(master,text="+",width=3,command=lambda:self.action('+')).grid(row=4, column=3)
 Button(master,text="x",width=3,command=lambda:self.action('x')).grid(row=2, column=3)
 Button(master,text="-",width=3,command=lambda:self.action('-')).grid(row=3, column=3)
 Button(master,text="÷",width=3,command=lambda:self.action(self.newdiv)).grid(row=1, column=3) 
 Button(master,text="%",width=3,command=lambda:self.action('%')).grid(row=4, column=2)
 Button(master,text="7",width=3,command=lambda:self.action('7')).grid(row=1, column=0)
 Button(master,text="8",width=3,command=lambda:self.action(8)).grid(row=1, column=1)
 Button(master,text="9",width=3,command=lambda:self.action(9)).grid(row=1, column=2)
 Button(master,text="4",width=3,command=lambda:self.action(4)).grid(row=2, column=0)
 Button(master,text="5",width=3,command=lambda:self.action(5)).grid(row=2, column=1)
 Button(master,text="6",width=3,command=lambda:self.action(6)).grid(row=2, column=2)
 Button(master,text="1",width=3,command=lambda:self.action(1)).grid(row=3, column=0)
 Button(master,text="2",width=3,command=lambda:self.action(2)).grid(row=3, column=1)
 Button(master,text="3",width=3,command=lambda:self.action(3)).grid(row=3, column=2)
 Button(master,text="0",width=3,command=lambda:self.action(0)).grid(row=4, column=0)
 Button(master,text=".",width=3,command=lambda:self.action('.')).grid(row=4, column=1)
 Button(master,text="(",width=3,command=lambda:self.action('(')).grid(row=2, column=4)
 Button(master,text=")",width=3,command=lambda:self.action(')')).grid(row=2, column=5)
 Button(master,text="√",width=3,command=lambda:self.squareroot()).grid(row=3, column=4)
 Button(master,text="x2",width=3,command=lambda:self.square()).grid(row=3, column=5)
#Main
root = Tk()
obj=calc(root) #object instantiated
root.mainloop()

• 200_success: For comparison, could you add a link to where you got the original code from?
• Riv: @200_success - per your request I included a full copy of the code I got from the web. The link is techinfected.net/2016/02/…
• 200_success: Our convention is to put original code that is not to be reviewed in blockquotes, which is what I did in Rev 6.
• Riv: Oh, I was wondering what happened, I thought I messed it up
• C. Harley: Any reason to pick Tk verses Kivy?

Answer 1

When I was learning Python, I found The Zen of Python quite helpful.

Formatting

I agree about renaming self.e to self.textbox. Descriptive names are generally better, unless this results in an overly long and unwieldy name. In addition to that there are a few more formatting issues. (You may find the PEP 8 Style Guide helpful)

• Redundant comments. (See https://www.python.org/dev/peps/pep-0008/#inline-comments) For example, in this line:

  obj = Calc(root) # object instantiated
  

  The comment is not particularly helpful here as we can see that Calc(root) clearly instanciates a new Calc object. The comment on the following line, on the other hand, is more helpful:

  self.txt = self.getvalue() # No need to manually delete when done from keyboard
  

• Method names that do not use underscores to separate words. For example, instead of stripfirstchar we should have strip_first_char

• While I could not find any mention of this in PEP 8, in my experience a class's __init__ method is almost always placed before any other methods.

• Use docstrings instead of comments to document entire functions. For example:

  def getvalue(self): # Returns value of the text box
   return self.e.get()
  

  becomes

  def getvalue(self):
   """Returns value of the text box"""
   return self.e.get()
  

• There is no need to wrap a method in a lambda when it can be used on its own. For example, this:

  Button(master, text='AC', width=3, command=lambda: self.clearall()).grid(row=1, column=4)
  

  can be rewritten as:

  Button(master, text='AC', width=3, command=self.clearall).grid(row=1, column=4)
  

• Use or instead of any when all conditions are known beforehand. For example:

  any([event.char.isdigit(), event.char in '/*-+%().'])
  

  can be rewritten as

  event.char.isdigit() or event.char in '/*-+%().'
  

Practical Issues

• The C button in the GUI does not work properly. This is because of an indentation issue in the method clear1. The call to self.refresh_text should be outside the else block.

• If I remove all characters in the text, then try to type something, the program will raise an IndexError. This can be fixed by changing the condition in the if statement in the strip_first_char method to

  len(self.txt) > 0 and self.txt[0] == '0'
  

• Open the window only if this program is being run as __main__. Check if __name__ == '__main__' before opening the window. This is to be sure that this will not happen if someone is trying to use this program as a library. (e.g. embedding this calculator in another application)

• % should be a special function. As it is, if I type 1%1, the program will interpret this as 1/1001 when it should cause some sort of syntax error. There are other ways to fix this, but this seems to be both the easiest to implement and the way most calculators I have seen handle this.

---

Using eval is usually a very bad idea

I see no way to remove eval from this code without significant changes. Letting the user type their math directly makes it harder to not use eval here, otherwise you could store the math in an internal, easy to parse form, and convert that to a more user-friendly string before displaying it, but this would require rewriting almost all of the program.