Python calculator

Question

I'd appreciate any comments about my Python calculator, ways in which it can be improved and where it's currently lacking.

import operator
"""
Do the actual calculation
"""
def calculator(operation, operand_1, operand_2):
 ops = {"+": operator.add, "-": operator.sub, "/": operator.div, "*": operator.mul}
 return ops[operation](operand_1, operand_2)
"""
Initiate the user interface and input from user
"""
def main():
 print_line(19)
 print("WELCOME to CALC+-/*")
 print_line(19)
 while True:
 operation = raw_input("Operation type: ")
 operand_1 = int(raw_input("Operand 1: "))
 operand_2 = int(raw_input("Operand 2: "))
 calculation = calculator(operation, operand_1, operand_2)
 print("Result: " + str(calculation))
 if raw_input("Enter to continue, or 'q' to quit: "):
 break
"""
Utility function to print dashed lines
"""
def print_line(line_length):
 print("-"*line_length)
main()

Answer 1

The docstring of a function should be at the top of the inside of a function.

For example this:

"""
Utility function to print dashed lines
"""
def print_line(line_length):
 print("-"*line_length) # Also, space the operators out.

should be:

def print_line(line_length):
 """Utility function to print dashed lines"""
 print("-" * line_length)

Hmmm:

print_line(19)
print("WELCOME to CALC+-/*")
print_line(19)

Why 19? I would probably create a variable called header and get it's length:

header = "WELCOME to CALC+-/*"
print_line(len(header))
print(header)
print_line(len(header))

So you aren't confused why you used 19 later on when you forget.

You should also have two newlines between functions.

This conditional:

if raw_input("Enter to continue, or 'q' to quit: "):
 break

Allows me to exit if I type anything including (but not limited to) q.

cont = raw_input("Enter to continue, or 'q' to quit: ")
while True:
 if cont == "":
 break
 elif cont == "q":
 return
 cont = raw_input("Invalid command, please try again: ")

You also have some other fragile things, if you input +<space> or any operand with spaces the program will crash.

replace the spaces with nothing:

operation = raw_input("Operation type: ").replace(" ", "")

Also, by convention you should do:

if __name__ == "__main__":
 main()

This answer illustrates some reasons why. Although I think it is unlikely this program will be imported by another program, you might as well be safe than sorry.

• EKons: @zondo What does int have to do with the operands?
• zondo: @Έρικ: Sorry, I was thinking of the numbers instead of the operand.
• EKons: @zondo The numbers alerady have int, no need to double-int 'em. int(operator) will crash the program, because operator is not an int.
• zondo: @Έρικ Κωνσταντόπουλος: It becomes an integer because of int(raw_input(...)). What I was saying is that raw_inputcan return something with spaces on the ends, and intwill still convert it just fine.

Answer 2

People usually don't think in polish notation,
but instead think in operand_1, operation, operand_2 order.

You may want to ask for input in that order (then you can call calculator() in whichever order you wish):

 operand_1 = int(raw_input("Operand 1: "))
 operation = raw_input("Operation type: ")
 operand_2 = int(raw_input("Operand 2: "))

• EKons: Human mind (at least mine) thinks like this: maths.operation.add(a,b). So, it first introduces a and b, and then the operation (add).
• Sunj Obb: I actually had it this way first time round but changed it as it felt more effortless this way, to get the operator out of the way before entering the numbers.

Answer 3

Review

import operator
"""
Do the actual calculation
"""
def calculator(operation, operand_1, operand_2):
 ops = {"+": operator.add, "-": operator.sub, "/": operator.div, "*": operator.mul}
 return ops[operation](operand_1, operand_2)

You import the whole module, when, in fact, you just need four of its functions. Result: you eat too much RAM.

Try doing dir(__import__('operator')). You will be really baffled because it has 126 functions (aside from the built-in module ones), while you need 4.

When having to work with lots of data, even the last byte of your RAM must be saved, so import less, eat less! It's a good practice to import less.

Here is a solution to the problem:

from operator import add, sub, div, mul
"""
Do the actual calculation
"""
def calculator(operation, operand_1, operand_2):
 ops = {"+": add, "-": sub, "/": div, "*": mul}
 return ops[operation](operand_1, operand_2)

---

"""
Do the actual calculation
"""
def calculator(operation, operand_1, operand_2):
 ops = {"+": operator.add, "-": operator.sub, "/": operator.div, "*": operator.mul}
 return ops[operation](operand_1, operand_2)
"""
Initiate the user interface and input from user
"""
def main():
 print_line(19)
 print("WELCOME to CALC+-/*")
 print_line(19)
 while True:
 operation = raw_input("Operation type: ")
 operand_1 = int(raw_input("Operand 1: "))
 operand_2 = int(raw_input("Operand 2: "))
 calculation = calculator(operation, operand_1, operand_2)
 print("Result: " + str(calculation))
 if raw_input("Enter to continue, or 'q' to quit: "):
 break
"""
Utility function to print dashed lines
"""
def print_line(line_length):
 print("-"*line_length)

I can see these are not docstrings, as @Dair believes; they are comments, because the main function also has one over it. Please use single-line comments instead (#comment):

#Do the actual calculation
def calculator(operation, operand_1, operand_2):
 ops = {"+": operator.add, "-": operator.sub, "/": operator.div, "*": operator.mul}
 return ops[operation](operand_1, operand_2)
#Initiate the user interface and input from user
def main():
 print_line(19)
 print("WELCOME to CALC+-/*")
 print_line(19)
 while True:
 operation = raw_input("Operation type: ")
 operand_1 = int(raw_input("Operand 1: "))
 operand_2 = int(raw_input("Operand 2: "))
 calculation = calculator(operation, operand_1, operand_2)
 print("Result: " + str(calculation))
 if raw_input("Enter to continue, or 'q' to quit: "):
 break
#Utility function to print dashed lines
def print_line(line_length):
 print("-"*line_length)

---

 print("-"*line_length)

Here, you seem to have missed the spaces. Here's the fix:

 print("-" * line_length)

---

print("WELCOME to CALC+-/*")

 print("Result: " + str(calculation))

print("-"*line_length)

It's not python-3.x here, so print is a statement, not a function!

It's not normal to refer to literals and whole expressions like (a), and it uselessly slows down code, especially with lots of data (i.e. [(i) for i in xrange(2562700000)]), so here is a fix:

print "WELCOME to CALC+-/*"

 print "Result: " + str(calculation)

print "-"*line_length

---

main()

You seem not to call the main function correctly. Here is how you should call it, because, otherwise, the program might get imported and run:

if __name__ == "__main__":
 main()

---

Some variable names are really large, others are too short/not clear enough.

---

 if raw_input("Enter to continue, or 'q' to quit: "):
 break

You would allow any input other than to exit your program here, not just q. The solution is here:

 if raw_input("Enter to continue, or 'q' to quit: ") == "q":
 break

I would also suggest supporting both Q and q, like this:

 if raw_input("Enter to continue, or 'q' to quit: ").lower() == "q":
 break

---

 print("Result: " + str(calculation))

There is string formatting for that!

 print("Result: %d" % calculation)

---

 print_line(19)
 print("WELCOME to CALC+-/*")
 print_line(19)

Whoa, wait a sec, what!? The len builtin can help you here!

 header = "WELCOME to CALC+-/*"
 print_line(len(header))
 print(header)
 print_line(len(header))

Result

from operator import add, sub, div, mul
#Do the actual calculation
def calc(op, a, b):
 ops = {"+": add, "-": sub, "/": div, "*": mul}
 return ops[op](a, b)
#Initiate the user interface and input from user
def main():
 header = "WELCOME to CALC+-/*"
 print_dashed_line(len(header))
 print header
 print_dashed_line(len(header))
 while True:
 op = raw_input("Operation type: ")
 a = int(raw_input("Operand 1: "))
 b = int(raw_input("Operand 2: "))
 r = calc(op, a, b)
 print "Result: %d" % r
 if raw_input("Enter to continue, or 'q' to quit: ").lower() == "q":
 break
#Utility function to print dashed lines
def print_dashed_line(length):
 print "-" * length
if __name__ == "__main__":
 main()

• Sunj Obb: Thanks a lot, you've picked up on quite a few things, appreciated :)
• EKons: @OmarTufayl I tried to make the code as good as it can be. I didn't agree with some of the opinions, I agreed with others, and I included my own. But, most importantly, each review section is based on the original code, not modified one.