1
\$\begingroup\$

Hello I made this calculator in Python, I know it's pretty basic but I want your opinions if I made it well or not, and if you find any mistakes or not.
P.S ( I know I could do another better than this one if I looked in internet but I want to evolve that's why I'm here )

#Commands
print("Operations that you can use:")
print("Addition: +")
print("Subtraction: -")
print("Multiplication: *")
print("Division: /")
print("Exponentiation: **")
print("Radicalization: **(1/")
print("\n")

#Variables
x = None
y = None
z = None
ce = 0

while ce < 1:

# Inputs
    x = float(input("First Value: "))
    y = input("Operation: ")
    z = float(input("Second Value: "))

# If
    if '+' == y:
        add = float(x) + float(z)
        print(add)
    elif '-' == y:
        sub = float(x) - float(z)
        print(sub)
    elif '/' == y:
        div = float(x) / float(z)
        print(div)
    elif "**(1/" == y:
        rad = float(x) **(1/float(z))
        print(rad)
    elif "**" == y:
        exp = float(x) ** float(z)
        print(exp)
    elif '*' == y:
        mul = float(x) * float(z)
        print(mul)
    print("Continue or Exit?")
    ce = int(input("    0    or   1  : "))
\$\endgroup\$
1
\$\begingroup\$

Let me offer the following rewrite of your program and then let's walk through it:

names = {'Addition' : '+',
         'Subtraction' : '-',
         'Division' : '/',
         'Exponentation': '**',
         'Radicalization': '**(1/'
        }

print("Operations that you can use:")
for op in names:
    print(op, names[op])

ops = {
       '+' : (lambda x, y: x + y),
       '-' : (lambda x, y: x - y),
       '*' : (lambda x, y: x * y),
       '/' : (lambda x, y: x / y),
       '**' : (lambda x, y: x ** y),
       '**(1/' : (lambda x, y: x ** 1.0 / y),
       'na' : (lambda x, y: 'Unknown binary operator')
      }

ce = 0
while ce != 1:
    x = float(input("First Value: "))
    y = input("Operation: ")
    z = float(input("Second Value: "))

    op = ops.get(y, 'na')
    print(ops[op](x, z))

    print("Continue or Exit?")
    ce = int(input("    0    or   1  : "))
  • It seems unnecessary to define x, y and z to be None so we just define them inside the main loop.

  • The ops dictionary stores a symbol of the operator and the actual implementation in a lambda function. It also stores a special 'na' value to represent any unknown operator; this is useful in the main loop where we return that key if we don't find what the user gave us as input.

  • The key difference and main idea here is a data-driven solution. That is, notice how we've put the supported operators into data structures. This way it's easier to make changes and to maintain the program, e.g., when you add a new binary operator, you don't have to touch the logic of your calculator, just add a new element into your data structure.

  • There's no explicit if-elif chain anymore, yay! Such chains are often annoying to write and it's always a potential code smell. No matter how experienced you are, it's one of those things where it's easy to make a human error and write something you didn't mean to, but it can be tough to discover the bug.

| improve this answer | |
\$\endgroup\$

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.