[Tutor] Fraction Class HELP ME PLEASE!

[Cameron Simpson]

On 06Aug2015 23:50, Quiles, Stephanie <stephanie.quiles001 at albright.edu> wrote:
>thanks Cameron! Here is what i have so far… new question… how do
>i test the iadd, imul, etc. operators?

Like the others, by firing them. You test __add__ by running an add between two 
expressions:

  F1 + F2

You test __iadd__ by running the augmented add operation:

  F1 += F2

and so forth. The "i" probably comes from the word "increment" as the commonest 
one of these you see is incrementing a counter:

  count += 1

They're documented here:

  https://docs.python.org/3/reference/datamodel.html#object.__iadd__

The important thing to note is that they usually modify the source object. So:

  F1 += F2

will modify the internal values of F1, as opposed to __add__ which returns a 
new Fraction object.

>Hopefully this time the
>indents show up.

Yes, looks good.

>And yes the beginning code came out of our text
>book I added on some functions myself but they give you a big chunk
>of it.

I thought it looked surprisingly complete given your questions. It's good to be 
up front about that kind of thing. Noone will think less of you.

>I am hoping that this is what the professor was looking for.
>I want to add some more operators but am unsure how to put them in
>or test them? For example I want to add __ixor__, itruediv, etc.
>Any other suggestions would be great!

Adding them is as simple as adding new methods to the class with the right 
name, eg:

    def __iadd__(self, other):
        ... update self by addition of other ...

If you want to be sure you're running what you think you're running you could 
put print commands at the top of the new methods, eg:

    def __iadd__(self, other):
        print("%s.__iadd__(%s)..." % (self, other))
        ... update self by addition of other ...

Obviously you would remove those prints once you were satisfied that the code 
was working.

Adding __itruediv__ and other arithmetic operators is simple enough, but 
defining __ixor__ is not necessarily meaningful: xor is a binary operation 
which makes sense for integers. It needn't have a natural meaning for 
fractions. When you define operators on an object it is fairly important that 
they have obvious and natural effects becauase you have made it very easy for 
people to use them.  Now, you could _define_ a meaning for xor on fractions, 
but personally that is one I would not make into an operator; I would leave it 
as a normal method because I would want people to think before calling it.

The point here being that it is generally better for a program to fail at this 
line:

  a = b ^ c     # XOR(b, c)

because "b" does not implement XOR than for the program to function but quietly 
compute rubbish because the user _thoght_ they were XORing integers (for 
example).

Added points: make your next reply adopt the interleaved style of this message, 
where you reply point by point below the relevant text. It makes discussions 
read like conversations, and is the preferred style in this list (and many 
other techincal lists) because it keeps the response near the source text. Hand 
in hand with that goes trimming irrelevant stuff (stuff not replied to) to keep 
the content shorter and on point.

Other random code comments:

[...snip: unreplied-to text removed here...]
>def gcd(m, n):
>    while m % n != 0:
>        oldm = m
>        oldn = n
>
>        m = oldn
>        n = oldm % oldn
>    return n

It reads oddly to have a blank line in the middle of that loop.

>class Fraction:
>    def __init__(self, top, bottom):
>        self.num = top
>        self.den = bottom
>
>    def __str__(self):
>        if self.den == 1:
>            return str(self.num)
>        elif self.num == 0:
>            return str(0)
>        else:
>            return str(self.num) + "/" + str(self.den)

While your __str__ function will work just fine, stylisticly it is a little 
odd: you're mixing "return" and "elif". If you return from a branch of an "if" 
you don't need an "elif"; a plain old "if" will do because the return will 
prevent you reaching the next branch. So that function would normally be 
written in one of two styles:

Using "return":

    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)

or using "if"/"elif"/...:

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

For simple things like __str__ the former style is fine. For more complex 
functions the latter is usually better because your code does not bail out half 
way through - the return from the function is always at the bottom.

>    def simplify(self):
>        common = gcd(self.num, self.den)
>
>        self.num = self.num // common
>        self.den = self.den // common

Again, I would personally not have a blank line in th middle of this function.

>    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)

Here is where the earlier discussion about __add__ versus __iadd__ comes into 
consideration. I would be definine __iadd__ here because it is closely related 
to __add__. It would look a lot like __add__ except that instead of returning a 
new Fraction it would overwrite .num and .den with the values for the new 
fraction.

If Addition were complex (and fractional addition is near this border for me) I 
might define a "private" called ._add to compute the new numerator and 
denominator, and then define __add__ and __iadd__ in terms of it, untested 
example:

    def _add(self, otherfraction):
        newnum = self.num * otherfraction.den + \
                 self.den * otherfraction.num
        newden = self.den * otherfraction.den
        common = gcd(newnum, newden)
        return newnum // common, newden // common

    def __add__(self, otherfraction):
        newnum, newden = self._add(otherfraction)
        return Fraction(newnum, newden)

    def __iadd__(self, otherfraction):
        newnum, newden = self._add(otherfraction)
        self.num = newnum
        self.den = newdem

You can see that this (a) shortens the total code and (b) guarentees that 
__add__ and __iadd__ perform the same arithmetic, so that they cannot diverge 
by accident.

The shared method _add() is a "private" method. In Python this means only that 
because its name begins with an underscore, other parts of the code (outside 
the Fraction class itself) are strongly discouraged from using it: you, the 
class author, do not promise that the method will not go away or change in the 
future - it is part of the arbitrary internal workings of your class, not 
something that others should rely upon.

This is a common convention in Python - the language does not prevent others 
from using it. Instead we rely on authors seeing these hints and acting 
sensibly. By providing this hint in the name, you're tell other users to stay 
away from this method.

>    def getNum(self):
>        return self.num
>
>    def getDen(self):
>        return self.den

These two methods are hallmarks of "pure" object oriented programming. A pure 
OO program never accesses the internal state of antoher object directly and 
instead calls methods like getNum() above to ask for these values. This lets 
class authors completely change the internals of a class without breaking 
things for others.

However, in Python it is more common to make the same kind of distinction I 
made earlier with the ._add() method: if an attribute like .num or .den does 
not have a leading underscore, it is "public" and we might expect other users 
to reach for it directly.

So we might expect people to be allowed to say:

    F1.num

to get the numerator, and not bother with a .getNum() method at all.

If there are other attributes which are more internal we would just name them 
with leaing underscores and expect outsiders to leave them alone.

>    def __gt__(self, otherfraction):
>        return (self.num / self.den) > (otherfraction.num / otherfraction.den)
>
>    def __lt__(self, otherfraction):
>        return (self.num / self.den) < (otherfraction.num / otherfraction.den)
>
>    def __eq__(self, otherfraction):
>        return (self.num / self.den) == (otherfraction.num / otherfraction.den)
>
>    def __ne__(self, otherfraction):
>        return (self.num /self.den) != (otherfraction.num /otherfraction.den)

These looke like normal arithmetic comparison operators.

I notice that you're comparing fractions by division. This returns you a 
floating point number. Floating point numbers are _not_ "real" numbers.  
Internally they are themselves implemented as fractions (or as scientific 
notation - a mantissa and an exponent - semanticly the same thing). In 
particular, floating point numbers are subject to round off errors.

You would be safer doing multiplecation, i.e. comparing:

    self.num * otherfraction.den == otherfraction.num * self.den

which will produce two integers. Python uses bignums (integers of arbitrary 
size) and some thing should never overflow, and is _not subject to round off 
errors.

When you use division for the comparison you run the risk that two Fractions 
with very large denominators and only slightly different numerators might 
appear equal when they are not.

>    def __is__(self, otherfraction):
>        return (self.num / self.den) is (otherfraction.num / otherfraction.den)

This is undesirable. There is no "__is__" method.

All the __name__ methods and attributes in Python are considered part of the 
language, and typically and implicitly called to implement things like 
operators (i.e. F1+F2 calls F1.__ad__(F2)).

You should not make up __name__ methods: they are reserved for the language.  
There are at least two downsides/risks here: first that you think this will be 
used, when it will not - the code will never be run and you will wonder why 
your call does not behave as you thought it should. The second is that some 
furture update to the language will define that name and it will not mean what 
you meant when you used it. Now your code _will_ run, but not do what a user 
expects!

BTW, the jargon for the __name__ names is "dunder": .__add__ is a "dunder 
method"; derived from "double underscore", which I hope you'd agree is a 
cumbersome term.

>def main():
>    F1 = Fraction(1,2)
>    F2 = Fraction(2,3)
>    print("F1 = ", F1)
>    print("F2 = ", F2)

print() adds a space between the comma separate items, you don't need to 
include one inside the quotes.

>    print("Add Fractions: F1 + F2=", Fraction.__add__(F1, F2))

Any reason you've not fired this implicitly? Like this:

    print("Add Fractions: F1 + F2=", F1 + F2)

Actually, you can make a case for doing both in your main program to show that 
they do the same thing.

>    print("Subtract Fractions: F1 - F2=", Fraction.__sub__(F1, F2))
>    print("Multiply Fractions: F1 * F2=", Fraction.__mul__(F1, F2))
>    print("True Division with Fractions: F1 / F2=", Fraction.__truediv__(F1, F2))
>    print("Exponentiation with Fractions: F1 // F2=", Fraction.__pow__(F1, F2))

Shouldn't this be "**"?

>    print("Is F1 Greater than F2?:", Fraction.__gt__(F1, F2))
>    print("Is F1 less than F2?:", Fraction.__lt__(F1, F2))
>    print("Is F1 Equal to F2?:", Fraction.__eq__(F1, F2))
>    print("Is F1 different than F2?:", Fraction.__ne__(F1, F2))
>    print ("Is F1 same as F2?:", Fraction.__is__(F1, F2))

Here you want to avoid this. Firstly, Python has an "is" operator, and it does 
not have a dunder method. Secondly, in what way does your __is__ differe from 
__eq__?

>    print("Is:", Fraction.__iadd__(F1, F2))
>
>if __name__ == '__main__':
>    main()

Otherwise this is all looking promising. Does it run correctly for you?

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