Fraction Class in Python

Question

I decided to train myself in OOP with a simple perfect precision Fraction class.

from __future__ import division
import doctest


class Fraction:
    """
    Implements a Fraction Class with perfect precision
    operations and utility methods. The built-in operators
    are overwritten to provide a more natural interface.
    """
    def __init__(self,num,den):
        if den == 0:
            raise ValueError("Denominator must not be zero.")

        self.num = num
        self.den = den

    @staticmethod
    def greatest_common_divisor(a,b):
        """
        Returns the greatest number 'n' existing such
        that a % n == 0 and b % n == 0.
        This number may be one if 'a' and 'b' are coprimes.

        >>> Fraction.greatest_common_divisor(20,15)
        5
        """
        def common(a,b):
            return [i for i in a if i in b]

        def div(n):
            return [i for i in range(1,n+1) if n % i == 0]

        return max(common(div(a),div(b)))

    @staticmethod
    def invert(fraction):
        """
        Returns a fraction where the numerator is the previous
        denominator and vice-versa.

        >>> Fraction.invert(Fraction(3,5))
        5/3
        """
        return Fraction(fraction.den,fraction.num)

    def from_string(text):
        """
        Generates a Fraction object from a string rapresentation
        of two integers seperated by '/'.

        >>> Fraction.from_string('4/9') + Fraction.from_string('2/18')
        5/9
        """
        return Fraction(*[int(i) for i in text.split('/')])

    def simplify(self):
        """
        Returns an eqivalent but simpler Fraction.

        >>> Fraction.simplify(Fraction(210,20))
        21/2
        """
        fact = self.greatest_common_divisor(self.num,
                                       self.den)
        return Fraction(self.num // fact, self.den // fact)

    def __mul__(self,fraction):
        """
        Fraction multiplication.

        >>> Fraction(4,3) * Fraction(1,20)
        1/15
        """
        return Fraction.simplify(
            Fraction(self.num*fraction.num,
                       self.den*fraction.den))

    def __add__(self,fraction):
        """
        Fraction addition.

        >>> Fraction(4,9) + Fraction(11,7)
        127/63
        """
        common_den = self.greatest_common_divisor(
            self.den,fraction.den)
        num1 = self.num * fraction.den
        num2 = fraction.num * self.den
        return Fraction.simplify(
            Fraction(num1+num2, fraction.den*self.den))

    def __sub__(self,fraction):
        """
        Fraction subtraction.

        >>> Fraction(1,2) - Fraction(1,3)
        1/6
        """
        return self + Fraction(-fraction.num,
                                fraction.den)

    def __truediv__(self,fraction):
        """
        Fraction division.

        >>> Fraction(4,8) / Fraction(9,2)
        1/9
        """
        return self * self.invert(fraction)

    def __repr__(self):
        """
        Returns a printable representation of the fraction
        that can also be fed back into the class via
        'Fraction.from_string'.
        This method is called automatically on printing.

        >>> Fraction(5,8)
        5/8
        >>> Fraction(2,9)
        2/9
        """
        return '{}/{}'.format(self.num, self.den)

def main():
    doctest.testmod()

if __name__ == "__main__":
    main()

Answer 1

The from_string() function is missing a @staticmethod decorator, and thus does not work in Python 2.

---

Your greatest_common_divisor() function works using brute force: it literally tests every possible divisor and picks the greatest one that is common to both. A much faster method is the Euclidean Algorithm.

@staticmethod
def gcd(a, b):
    while b:
        a, b = b, a % b
    return a

---

Validation and error handling could be tightened up a bit.

• The rejection of floating-point arguments is inconsistent:

  >>> Fraction(3.14, 1)
  3.14/1
  >>> Fraction.from_string('3.14/1')
  Traceback (most recent call last):
    […]
    File "fraction.py", line 55, in <listcomp>
      return Fraction(*[int(i) for i in text.split('/')])
  ValueError: invalid literal for int() with base 10: '3.14'
  

• Instantiating a Fraction with strings initially appears to work, but fails in an odd way later. Either both of these statements should work, or both should fail.

  >>> Fraction('1', '3')
  1/3
  >>> Fraction('1', '3') + Fraction('2', '3')
  Traceback (most recent call last):
    […]
    File "fraction.py", line 32, in div
      return [i for i in range(1,n+1) if n % i == 0]
  TypeError: Can't convert 'int' object to str implicitly
  

• In Fraction.from_string(), the abstraction is slightly leaky. I would prefer to see a ValueError instead of the following:

  >>> Fraction.from_string('1/3/5')
  Traceback (most recent call last):
    File "<stdin>", line 1, in <module>
    File "fraction.py", line 55, in from_string
      return Fraction(*[int(i) for i in text.split('/')])
  TypeError: __init__() takes 3 positional arguments but 4 were given
  

• Personally, I would choose to raise a ZeroDivisionError if the denominator is 0.

  >>> Fraction(1, 0)
  Traceback (most recent call last):
    File "<stdin>", line 1, in <module>
    File "fraction.py", line 13, in __init__
      raise ValueError("Denominator must not be zero.")
  ValueError: Denominator must not be zero.
  

Answer 2

From the docs:

[__repr__] should look like a valid Python expression that could be used to recreate an object with the same value (given an appropriate environment).

Your current __repr__ should really be renamed __str__, and replaced by something like:

 def __repr__(self):
     return 'Fraction({0.num}, {0.den})'.format(self)

---

invert needs access to the class, so should probably be a @classmethod rather than @staticmethod:

@classmethod
def invert(cls, fraction):
    """
    Returns a fraction where the numerator is the previous
    denominator and vice-versa.

    >>> Fraction.invert(Fraction(3, 5))
    5/3
    """
    return cls(fraction.den, fraction.num)

This factors out the explicit class name from the function, making it work better with any future inheritance. Note that I've also added some extra spaces; see the style guide. You could alternatively implement it as a standard instance method, then call it like Fraction(3, 5).invert().

Similarly, you can remove the explicit class from from_string by making it a class method (currently it's an instance method without a self parameter, so won't work in Python 2.x):

@classmethod
def from_string(cls, text):
    """
    Generates a Fraction object from a string rapresentation
    of two integers seperated by '/'.

    >>> Fraction.from_string('4/9') + Fraction.from_string('2/18')
    5/9
    """
    return cls(*[int(i) for i in text.split('/')])

You can also simplify with e.g. cls(*map(int, text.split('/')).

Where you return a new Fraction from an instance method, you can access the class less explicitly with self.__class__(...).

---

You don't need to implement greatest_common_divisor yourself; Python has fractions.gcd.