This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author belopolsky
Recipients belopolsky, mark.dickinson
Date 2010-05-25.16:11:04
SpamBayes Score 0.008627068
Marked as misclassified No
Message-id <1274803866.96.0.689884701896.issue8817@psf.upfronthosting.co.za>
In-reply-to
Content
Just a few nitpicks on the patch (in increasing pickiness):

1. Any reason to prefer PyTuple_SetItem to PyTuple_SET_ITEM at the end of _PyLong_Divmod_Near?   You are filling a brand new tuple, so PyTuple_SET_ITEM seems to be more appropriate.

2. temp = (quo_is_neg ? long_add : long_sub)(..) is clever, but IMO is less readable than 

if (quo_is_neg)
   temp = long_add(..)
else
   temp = long_sub(..)

The later form may also be more optimization friendly, particularly if compiler wants to inline static long_add or long_sub.

3. Given that arguments are named 'a' and 'b' it is a bit confusing to have local variable c of a different type.  I think 'cmp' would be a better choice.

4. I see that you removed a comment that displays round() implemented in python.  I think it would be helpful to preserve it for documentation and testing purposes even though the actual algorithm is slightly different.  As long as the results are the same, it is helpful to have reference python code.

5. Similarly to #4, having python implementation of divmod_near() displayed somewhere will be helpful.
History
Date User Action Args
2010-05-25 16:11:07belopolskysetrecipients: + belopolsky, mark.dickinson
2010-05-25 16:11:06belopolskysetmessageid: <1274803866.96.0.689884701896.issue8817@psf.upfronthosting.co.za>
2010-05-25 16:11:05belopolskylinkissue8817 messages
2010-05-25 16:11:04belopolskycreate