5brickbomzh
5brickbomzh

Reputation: 99

Find the greatest common divisor

def gei(a, b):
     '''
     a, b: only positive integers! If you don't, we will make you. Max 1337
     For extracting the juice of the numbers in the form of a common divider
     '''
     #Not Ruby, no to_i, but int()
     smallest = int(abs(min(a, b)))
     biggest = int(abs(max(a, b)))
     print "You inputed: ", smallest, " and ", biggest, " their order doesn't matter."
     print "Do you want to see guess numbers? Type 'yes', if you do! "
     selection = raw_input()
     print
     print
     #To evade infinite loops and too big numbers, we use count.
     count = 0
     ans = smallest
     truth = str(selection) == str('yes')
     def condition(ans, base):
         ans1 = base % ans == 0
         return ans1
     while condition(ans, biggest) == False or condition(ans, smallest) == False:
         ans -= 1
         count += 1
     if count >= 1337:
         break
     elif truth == True:
         print  ans
     if truth == True:
         print
         print
     print  "After weeks of calculation, here is your greater common divider: "
     return ans

So yeah, 8th grade informatics assignment to extract common greater dividers. I was wondering, maybe you guys know how can I make it less cumbersome? How to avoid using definition inside and naming so many variables?

Upvotes: 1

Views: 5879

Answers (4)

Sriwantha Attanayake
Sriwantha Attanayake

Reputation: 7908

Use Euclid's algorithm

int GCD(int a,int b){
 if(b==0){
          return a;
 }
 return GCD(b,a%b);        
}

Upvotes: 1

Blckknght
Blckknght

Reputation: 104712

A few things can be improved in your code.

First off, truth is a truly poor variable name, since it doesn't really tell you what it's contents mean. I'd use something like show_work instead.

Next, you have an internal function that tells you a variable divides another evenly (returning a Boolean value). I'd suggest just using the modulus value directly, rather than coverting it to a bool with == 0, and you also don't need it to be in a function. Also, it's never necessary to compare a value to True or False. Just use the value itself (or use not to invert it). Putting these together will make your while loop's condition not biggest % ans and not smallest % ans.

Finally, you can use a better algorithm than trying every value one by one. Euclid's algorithm is a great way to calculate the Greatest Common Divisor quickly, and it's very easy to implement in Python (but I'll leave it to you).

Upvotes: 0

root
root

Reputation: 80346

import fractions
print fractions.gcd(4,8)
>>> 4

But you can also look at the source:

def gcd(a, b):
    """Calculate the Greatest Common Divisor of a and b.

    Unless b==0, the result will have the same sign as b (so that when
    b is divided by it, the result comes out positive).
    """
    while b:
        a, b = b, a%b
    return a

Reference: http://en.wikipedia.org/wiki/Euclidean_algorithm

Upvotes: 8

Ber
Ber

Reputation: 41813

This is nice an consise, and names no variable:

def gcd(a,b):
    return max(d for d in xrange(1, min(a, b)+1) if a % d == b % d == 0)

print gcd(15, 25)

>>> 5

But please don't claim it as your own :)

Upvotes: -1

Related Questions