Java:: Why isn't this implementation of Fibonacci sequence with memoization not work?

Question

I have a quick question about implementing the Fibonacci sequence using the memoization (DP). I am using a HashTable, but for some reason, the table never seems to contain the elements. I inserted a print statement to print out something whenever a value was read from the hashtable and it seems like this never happens. I feel like its a simple fix, but I don't see it.

  public static int getFib(int n) {
        HashMap<Integer, Integer> dictionary = new HashMap<Integer, Integer>();
        if (n <= 2)
            return 1;
        else if (dictionary.containsKey(n)) {
            System.out.println("Reading From Table");
            return dictionary.get(n);
        } else {
            int val = getFib(n - 1) + getFib(n - 2);
            dictionary.put(n, val);
            return val;
        }
    }

(-/ Starts with double colon, double negation and no continuous form in the title /-). Pertaining to code: in declarations, prefer interfaces over implementations. Avoid one hit on the Map: use a variable result = dictionary.get(n); if (null != result) … — greybeard
– greybeard, Commented Jan 1, 2015 at 9:37

Eric Stein · Accepted Answer · 2015-01-01 04:55:29Z

5

You're calling getFib() recursively, and instantiating a new dictionary with each call. Make the dictionary a class-level variable.

answered Jan 1, 2015 at 4:55

Eric Stein

13.8k3 gold badges39 silver badges51 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Sridhar DD · Accepted Answer · 2015-01-01 05:04:18Z

4

dictionary is a local variable so the scope of this variable is within the function getFib.

If you call the function getFib recursively then each time the hash map will be created and instantiated and the scope of the hashmap dictionary will end on returning from the function.

You can have a global variable instead to solve this problem.

edited Jan 1, 2015 at 5:04

answered Jan 1, 2015 at 4:57

Sridhar DD

1,9801 gold badge11 silver badges17 bronze badges

Comments

Jobs · Accepted Answer · 2015-01-01 05:02:18Z

3

Initialize the dictionary outside your method. Right now, you are creating a new 'dictionary' in each recursive call.

edited Jan 1, 2015 at 5:02

answered Jan 1, 2015 at 4:56

Jobs

3,3878 gold badges30 silver badges57 bronze badges

Comments

Hunter Zhao · Accepted Answer · 2015-01-01 04:59:56Z

2

Make the dictionary a member variable rather than a local variable

private HashMap<Integer, Integer> dictionary = new HashMap<Integer, Integer>();

public static int getFib(int n) {

    // some operations
}

answered Jan 1, 2015 at 4:59

Hunter Zhao

4,6692 gold badges28 silver badges39 bronze badges

Collectives™ on Stack Overflow

Java:: Why isn't this implementation of Fibonacci sequence with memoization not work?

4 Answers 4

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related