[Tutor] Fraction Class HELP ME PLEASE!

[Cameron Simpson]

On 07Aug2015 03:09, Quiles, Stephanie <stephanie.quiles001 at albright.edu> wrote:
>Hello again Cameron,
>
>Thank you, your reply is most helpful. Responses and updated code
>below. Thoughts would be appreciated. The assignment has been sent
>as my deadline was 11pm EST. But i was able to implement your
>suggestions and tips right before i submitted. responses are below
>and here is the final code i submitted for grading.

Hmm. Did you program run with successful tests? because there are some 
problems...

>    def __str__(self):
>        if self.den == 1:
>            return str(self.num)
>        if self.num == 0:
>            return str(0)
>        return str(self.num) + "/" + str(self.den)

Nicer to the eye. Thanks.

>    def simplify(self):
>        common = gcd(self.num, self.den)
>        self.num = self.num // common
>        self.den = self.den // common
>
>    def show(self):
>        print(self.num, "/", self.den)
>
>    def __add__(self, otherfraction):
>        newnum = self.num * otherfraction.den + self.den * otherfraction.num
>        newden = self.den * otherfraction.den
>        common = gcd(newnum, newden)
>        return Fraction(newnum // common, newden // common)

In principle you could change the last two lines above to:

    F = Fraction(newnum, newden)
    F.simplify()
    return F

That would let you reuse the code from the simplify() method, eliminating risk 
of not getting those two things the same (and therefore getting one wrong).

Alternatively, you could have the .__init__() method call .simplify() as its 
final action, meaning you would not need to worry about it when constructing a 
new Fraction. However, that is a policy decision - perhaps you want a Fraction 
to have exactly the original numerator/denominator unless the user explicitly 
wishes otherwise.

>    def __iadd__(self, otherfraction):
>        if isinstance(otherfraction):

isinstance() takes two arguments: the object being examines and a class to see 
if the object is an instance of the class. BTW, why are you doing this test? I 
would guess it is so that you can have otherfraction be either a Fraction or 
some other kind of numeric value.

>            return self__iadd__(otherfraction)

This isn't right either. No "." for one thing. It is syntacticly value, as it 
tries to call a function named "self__iadd__". But it will fail it runtime.

It looks to me like this is incomplete. This function looks like you want to 
handle a Fraction or a numeric value. However, the half for the non-Fraction 
value is not present and the half for the Fraction is wrong.

>    def __sub__(self, otherfraction):
>        newnum = self.num * otherfraction.den - self.den * otherfraction.num
>        newden = self.den * otherfraction.den
>        common = gcd(newnum, newden)
>        return Fraction(newnum // common, newden // common)
>
>    def __mul__(self, otherfraction):
>        newnum = self.num * otherfraction.num * self.den * otherfraction.den
>        newden = self.den * otherfraction.den
>        common = gcd(newnum, newden)
>        return Fraction(newnum // common, newden // common)

Here you're doing the greatest common factor thing again. This adds to the 
argument for reusing the .simplify() method here instead of wring out that 
arithmetic again. This is a situation you will encounter often, and it often 
gets called "DRY": don't repeat yourself. Just some more terminology.

The other benefit to embedding this in .simplify() is that someone reading the 
code (including yourself much later) can see this:

    F.simplify()

and know what's happening. Otherwise they need to read the arithmetic and 
figure out what it is for.

>    def __imul__(self, otherfraction):
>        if isinstance(otherfraction):
>            return self__mul__(otherfraction)

Same issues here as with __iadd__.

[...] snip ...]
>    def __rshift__(self, otherfraction):
>        return (self.num * self.den) >> (otherfraction.num * otherfraction.den)

This, like __xor__, is another binary operator. You can certainly define it.  
But does it make sense? Again, I would personally be reluctant to define 
__xor__ on Fractions.

>""" This program will test the following operators, addition, subtraction, multiplication,
>division, true division, exponentiation, radd, modulo, shifting, ordering (less than, greater , equal to, different than,
>same as). All using fractions. """

I would put this docstring at the top of the program, and possibly the main() 
function as well.

Actually, I would do two things: have a nice big opening comment or docstring 
at the top of the program briefly describing the Fraction class and that this 
mentioning program can be invoked as a "main program". The second thing is that 
the docstring above, because it reads as describing the main() function, should 
go _inside_ the main function. Like this:

    def main():
        ''' This program will test the following operators, addition, ...
            ........
        '''
        F1 = ...
        ...

When you put a string inside a function or class, immediately under the "def" 
or "class" line, that string is called a docstring as it is intended as 
documentation for that function or class. The Python language actually collects 
it and saves it as the .__doc__ attribute of the function or class, and you can 
get it back later.

In the interactive Python interpreter (which you get when you just run the 
command "python"), you can call the help() function on something and have the 
docstring recited back at you. So at the python prompt you can type:

    help(main)

and be told about it. By putting docstrings on your Fraction class and also on 
each of its methods you make that information available via help() later.

Otherwise, good luck.

Cheers,
Cameron Simpson <cs at zip.com.au>