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="x²", 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="x²",width=3,command=lambda:self.square()).grid(row=3, column=5)

#Main

root = Tk()

obj=calc(root) #object instantiated

root.mainloop()

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 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.