Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Below is the problem taken from here.

Question 10: GCD*

The greatest common divisor of two positive integers a and b is the largest integer which evenly divides both numbers (with no remainder). Euclid, a Greek mathematician in 300 B.C., realized that the greatest common divisor of a and b is one of the following:

the smaller value if it evenly divides the larger value, OR the greatest common divisor of the smaller value and the remainder of the larger value divided by the smaller value In other words, if a is greater than b and a is not divisible by b, then

gcd(a, b) == gcd(b, a % b)

Write the gcd function recursively using Euclid's algorithm.

def gcd(a, b):
    """Returns the greatest common divisor of a and b.
    Should be implemented using recursion.

    >>> gcd(34, 19)
    1
    >>> gcd(39, 91)
    13
    >>> gcd(20, 30)
    10
    >>> gcd(40, 40)
    40
    """
    "*** YOUR CODE HERE ***"

Solution:

def gcd(a, b):
    """Returns the greatest common divisor of a and b.
    Should be implemented using recursion.

    >>> gcd(34, 19)
    1
    >>> gcd(39, 91)
    13
    >>> gcd(20, 30)
    10
    >>> gcd(40, 40)
    40
    """
    if b > a:
        if b % a == 0:
            return a
        else:
            return gcd(b % a, a)
    else:
        if a % b == 0:
            return b
        else:
            return gcd(b, a % b)        

How can I improve this nested if block?

share|improve this question

1 Answer 1

up vote 7 down vote accepted

One of the nice things with recursion is that you can use it for argument handling.

Where you have a conditional to handle the b > a condition, you could instead use recursion.....

def gcd(a, b):
    """Returns the greatest common divisor of a and b.
    Should be implemented using recursion.

    >>> gcd(34, 19)
    1
    >>> gcd(39, 91)
    13
    >>> gcd(20, 30)
    10
    >>> gcd(40, 40)
    40
    """
    if b > a:
        return gcd(b, a)

    if a % b == 0:
        return b

    return gcd(b, a % b)        
share|improve this answer
    
So you are making sure first argumentt is bigger than second –  overexchange Jul 2 at 3:11
    
Yes, and note I also now removed the else. It was redundant –  rolfl Jul 2 at 3:15
4  
If b > a then a % b = a, and the first recursive call will be equivalent to swapping the input arguments, so you don't really need that first branch at all. Also, if you let the recursion go one level deeper, you don't need to compute a % b twice, or store it in a temp variable, but can simply do: def gcd(a, b): return gcd(b, a % b) if b else a. –  Jaime Jul 2 at 4:48

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.