Python : Good practices : Classes : Emulating fractions

Question

(First of all yes, I do know that a Fraction module exists, but I'm doing my own to practice!) My question is, having this code:

class Fraction(object):
    def __init__(self,num,den=1,reduce=True):
        # only accept integers
        if not(type(num) == int and type(den) == int):
            raise RuntimeError("You must pass integers as numerator \
and denominator!")
        # don't accept fractions with denominator 0
        if den == 0:
            raise ZeroDivisionError("The denominator must not be 0")
        # if both num and den are negative, flip both
        if num < 0 and den < 0:
            num = abs(num)
            den = abs(num)
        # if only the den is negative, change the "-" to the numerator
        elif den < 0:
            num *= -1
            den = abs(den)
        self.num = num
        self.den = den
        # the self.auto is a variable that will tell us if we are supposed to
        #automatically reduce the Fraction to its lower terms. when doing some
        #maths, if either one of the fractions has self.auto==False, the result
        #will also have self.auto==False
        self.auto = reduce
        if self.auto:
            self.reduce()

    def reduce(self):
        '''used to reduce the fraction to its lower terms using Euclid's\
Algorith'''
        a = self.num
        b = self.den
        # With the Euclid's Algorithm, the GCD of A and B, is B if B divides
        #evenly A. Otherwise, make B the new A, and make B the remainder of A/B!
        #e.g.(30,20). 30/20 is not integer... (30-20=10)
        # 20/10 = integer! 10 is the GCD of 20 and 30
        while a%b != 0:
            olda = a
            oldb = b
            a = oldb
            b = olda%oldb
        self.num //= b
        self.den //= b

    def __add__(self,other):
        '''addition implementation'''
        if type(other) == int:
            other = Fraction(other,1)
        elif type(other) == float:
            return NotImplemented
        num = self.num*other.den + self.den*other.num
        den = self.den * other.den
        return Fraction(num,den,self.auto and other.auto)

    def __radd__(self,other):
        '''raddition implemented enables integer + fraction statements'''
        # other is always a Fraction!! Well, we are in R(ight)__add__, so this
        #was called because the thingy on the right IS a Fraction
        # In this case we have to manipulate the "self" argument
        if type(self) == int:
            self = Fraction(self,1)
        elif type(self) == float:
            return NotImplemented
        num = self.num*other.den + self.den*other.num
        den = self.den * other.den
        return Fraction(num,den,self.auto and other.auto)

would you say it is good practice to, when implementing the __radd__ method, just call the __add__ inverting the arguments?? Or is it better practice to do it like so? (I assume the answer to __add__ and __radd__ applies to all other mathematical functions...) I have loads of code already, but for the sake of brevity I though only this was enough... (Also, can anyone tell me if stackoverflow has something like spoiler tags? You know, to write stuff inside that but without it showing)

Answer 1

In general, writing two pieces of code that do the same thing is poor practice. If two operations, in this case add to another object when self is on the left and add to another object when self is on the right, ultimately perform the same operation it would be best practice to simply have __radd__ call __add__ and return the results of that. This way if you modify the behaviour of fraction addition, for example if you wanted to add the ability to use floats, you only need to modify code in one place instead of two.

Do note that you don't need to reverse the arguments; whether you use __radd__ or __add__, self is always the first argument. In the docs the example is given x - y where x does not implement subtraction. The call made is y.__rsub__(x), so self will always be your object. So you still need to check the type of other, since self is the one that will always be a fraction. (see: Documentation for __radd__)

My implementation, assuming all else the same would be:

    def __add__(self, other):
        # however you want to implement __add__

    def __radd__(self, other):
        return self.__add__(other)

Edit: Note that this only works if operand order doesn't matter. For subtraction, you would have to convert other to a fraction first, and then call __sub__ on that.

    def __sub__(self, other):
        # however you implement __sub__

    def __rsub__(self, other):
        if type(other) != Fraction:
            if type(other) == int:
                other = Fraction(other, 1)
            else:
                return NotImplemented
        return other.__sub__(self)

Also, for safe use with all types, you may want have an else: return NotImplemented or raise an error if the type is something other than float or int. Right now your class's behaviour with other object types is undefined. For example if I have an object with properties named num and den, but but mine are are a float and a string respectively, the error your code will give me is "you must pass integers as numerator and denominator", from the constructor, rather than something more intuitive like "Adding type Fraction with type MyType is not implemented".

Answer 2

Note that when you do 1 + myFraction, Python first tries to do int.__add__(1, myFraction), but since this is not implemented, it calls Fraction.__radd__(myFraction, 1), so with the reversed arguments.

I guess it is best practice, if you care about maintainable code, to normalize arguments in __radd__, and then call __add__ on the converted arguments. In this way, you do not duplicate the code needed to do the addition. Something like this:

def __radd__(self, other):
    if type(other) == int:
        other = Fraction(other, 1)
    elif type(other) == some_other_format_I_can_convert_to_Fraction:
        other = some_conversion(other)
    else:
        raise NotImplemented
    return self.__add__(other)

You could even skip the conversion to Fraction and pass other directly to __add__, since that function already knows ho to do the conversion. If you care about speed instead of code beauty, you might do the calculation directly in __radd__. But in this case, the math to do the addition will be duplicated, so if you discover an error, you have to remember to fix it in 2 places.