current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 2 down vote favorite
1

Problem:

Using the recursion approach, find a Fibonacci sum without repetition of computation.

def sum_fibonacci(n):
    """Compute the nth Fibonacci number.

    >>> sum_fibonacci(35)
    9227465
    >>> sum_fibonacci(10)
    55
    >>> sum_fibonacci(0)
    0
    >>> sum_fibonacci(1)
    1
    >>> sum_fibonacci(5)
    5
    """
    """Your code here"""

Solution:

fibcache = {}
def  sum_fibonacci(n):
    """Compute the nth Fibonacci number.

    >>> sum_fibonacci(35)
    9227465
    >>> sum_fibonacci(10)
    55
    >>> sum_fibonacci(0)
    0
    >>> sum_fibonacci(1)
    1
    >>> sum_fibonacci(5)
    5
    """
    if n == 0:
        fibcache[n] = 0
        return fibcache[n]
    elif n == 1:
        fibcache[n] = 1
        return fibcache[n]
    else:
        sum_left = 0
        sum_right = 0
        if n-2 in fibcache.keys():
            sum_left += fibcache[n-2]
        else:
            sum_left += sum_fibonacci(n-2)
            fibcache[n-2] = sum_left
        if n-1 in fibcache.keys():
            sum_right += fibcache[n-1]
        else:
            sum_right += sum_fibonacci(n-1)
            fibcache[n-1] = sum_right
        return sum_left + sum_right

This program uses dictionary data model amidst tree recursion.

Can it be more readable? Can it avoid global cache fibcache update? Because nonlocal is better than global.

Note: I'm currently aware of data models - class 'tuple', class 'list' and class 'dict'.

share|improve this question
    
Is this an exercise from a programming course? If so, can you link to the original problem statement? – Gareth Rees Jul 2 '15 at 9:56
2  
Your edit makes nonsense of my answer: see what you may and may not do after receiving answers. – Gareth Rees Jul 2 '15 at 12:48
up vote 7 down vote

I think that caching should look the same on every function,

cached_f(args):
    if args not in cache:
        cache[args] = f(args)
    return cache[args]

So Fibonacci becomes:

cache = {}    
def fib(n):
    if n not in cache.keys():
        cache[n] = _fib(n)
    return cache[n]

def _fib(n):
    if n < 2:
        return n
    else:
        return fib(n-1) + fib(n-2)

I'm not sure why the cache should not be global (other than namespace pollution), you could end with duplication of the results and also missing a cached result making you compute again what you wanted to avoid computing.

Also, you may initialize the cache with the base cases and skip them when writing the recursion, but that is not so clean.

share|improve this answer
up vote 2 down vote

If you don't like global variables (which you really shouldn't!), you can create a static variable by making it an attribute of the function:

def fib(n):
    if n in fib.cache:
        return fib.cache[n]
    ret = fib(n-2) + fib(n-1)
    fib.cache[n] = ret
    return ret
fib.cache = {0: 1, 1: 1}

Memoization is one of the poster childs of function decorators in Python, so an alternative approach would be something like:

class Memoize(object):
    def __init__(self, func):
        self.func = func
        self.cache = {}
    def __call__(self, *args):
        if args in self.cache:
            return self.cache[args]
        ret = self.func(*args)
        self.cache[args] = ret
        return ret

@Memoize
def fib(n):
    if n < 2:
        return 1
    return fib(n-2) + fib(n-1)
share|improve this answer
up vote 2 down vote

Memoization is not strictly needed to avoid to repeat computations

def fib(n):
    (x,y) = fibpair(n)
    return y

def fibpair(n):
    if n == 0:
       return (1,0)
    else:
       (x, y) = fibpair(n-1)
       return (x+y, x)

The functions are linked by the relation

fibpair(n) == (fib(n+1), fib(n))

Edit: if you dont like the idea of computing also fib(n+1) when you need fib(n), you can also start from

fp(n) == (fib(n), fib(n-1))

with a fictious value of 1 for fib(-1) to preserve the recurrence relation.

def fib(n):
    (x, y) = fp(n)
    return x    
def fp(n):
    if n==0:
        return (0, 1)
    else:
        (x,y) = fp(n-1)
        return (x+y, x)
share|improve this answer
    
Are you computing fib(n+1) too? Also, using a loop seems clearer than that if you won't use memoization – Dietr1ch Jul 3 '15 at 0:33
    
1) This is essentially the same as the classical iterative solution, where the loop is is represented by terminal recursion. But recursion was required, so... 2) Yes it computes an extra value. We can also start from a fictious value fib(-1) = 1. Or add tests. See edit. – Michel Billaud Jul 3 '15 at 8:09
    
I know it's the same, but it's definitely harder to understand for newcomers. It's also harder to generalize for other kind of recursions, as you have to state how the previous values will be needed. – Dietr1ch Jul 3 '15 at 14:06
    
Welll in fact it depends how much newcomers have been exposed to imperative programming. They have to twist their mind first to adapt to iterative logic. And then you ask them to see things recursively.... But simple algebra suffices here : fp(n) = (fib(n), fib(n-1)) = (fib(n-1)+fib(n-2), fib(n-1)) by definition of fib. And thus fp(n) = (x+y, x) where (x,y)=fp(n-1). – Michel Billaud Jul 3 '15 at 14:34

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.