2
\$\begingroup\$

Question:

Create a basic calculator using classes that given 3 inputs - number 1, number 2 and operator - calculates the result.

class calculator():

    def __init__(self,value1 , value2, operand):
        self.value1 = int(value1)
        self.value2 = int(value2)
        self.operand = operand

    def calculate (self):
        if self.operand == '+' :
            return self.value1 + self.value2

        if self.operand == '*':
            return self.value1 * self.value2

        if self.operand == '-' :
            return self.value1 - self.value2

        if self.operand == '/' :
            if self.value2 == 0:
                raise ZeroDivisionError
            else:
                return self.value1 / self.value2


r = calculator(6,7, '+')
print r.calculate()

r = calculator(3,7, '-')
print r.calculate()

r = calculator(6,0, '*')
print r.calculate()

r = calculator(9,5, '/')
print r.calculate()

How can i make this function better? Is this a correct implementation of a class?

\$\endgroup\$
1
  • 4
    \$\begingroup\$ What's the advantage of using a class like that, rather than using the math statements directly? Can you give us a use case/context where you plan to use it? \$\endgroup\$ Feb 22 '18 at 18:08
2
\$\begingroup\$

As a rule, if a class has two methods, one of which is the constructor, then it shouldn't be written as a class. A simple calculate(value1, value2, operand) function would be better.

If you do write a class, then by PEP 8 conventions it should be named Calculator instead.

I don't recommend the int() casts. print calculator(1.5, 3, '+').calculate() would result in 4, because int() performs truncation. A good design would either produce 4.5 as expected, or raise an exception on validation failure.

\$\endgroup\$
2
\$\begingroup\$
  • There is no need to manually raise ZeroDivisionError. Python will do it for you anyway.

  • Rather than inspecting an operand with the bunch of ifs, create a dictionary of operand, lambda:

    ops = {
        '+': lambda x,y: x + y,
        ....
    }
    

    and calculate the result as

    ops[operand](value1, value2)
    

    Such dictionary (initialized in a constructor) is the only justification for making a class. It doesn't feel right to make an operator and values class members. Consider

    r = calculator()
    r.calculate(1, 2, '+')
    r.calculate(3, 4, '*')
    

    etc.

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