On 07Aug2015 03:09, Quiles, Stephanie <stephanie.quiles...@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 <c...@zip.com.au>
_______________________________________________
Tutor maillist  -  Tutor@python.org
To unsubscribe or change subscription options:
https://mail.python.org/mailman/listinfo/tutor

Reply via email to