Skip to main content
Recommend tighter temporary variable scoping
Source Link
200_success
  • 144.2k
  • 22
  • 188
  • 473

This comment is inaccurate:

/*
 * TimeComplexity: O(n)
 * Space Complexity: http://www.geeksforgeeks.org/g-fact-86/
 */
public static int fibo(int n) { … }

A naïve recursive fibo() has O(2n) time complexity. Think of it this way: to calculate fibo(n), you break it up into two problems, each of size n - 1.

In fiboBottomUp(), don't declare/define int a = 0, since it is only ever used as a temporary variable inside the for-loop.

public static int fiboBottomUp(int n) {
    if (n < 0) {
        // Message is inaccurate: n = 0 is allowable
        throw new IllegalArgumentException("The value of n: " + n  + " should be non-negative.");
    }

    int b = 1;
    int c = 0;
    for (int i = 0; i < n; i++) {
        int a = b;
        b = c;
        c = a + b;
    }
    return c;
}

Using a HashMap<Integer, Integer> for fiboCache is more complicated than necessary. An ArrayList<Integer> or even an int[n + 1] will do, since all the keys are consecutive integers.

Consider widening your return types to long to stave off overflow for a while longer.

This comment is inaccurate:

/*
 * TimeComplexity: O(n)
 * Space Complexity: http://www.geeksforgeeks.org/g-fact-86/
 */
public static int fibo(int n) { … }

A naïve recursive fibo() has O(2n) time complexity. Think of it this way: to calculate fibo(n), you break it up into two problems, each of size n - 1.

Using a HashMap<Integer, Integer> for fiboCache is more complicated than necessary. An ArrayList<Integer> or even an int[n + 1] will do, since all the keys are consecutive integers.

Consider widening your return types to long to stave off overflow for a while longer.

This comment is inaccurate:

/*
 * TimeComplexity: O(n)
 * Space Complexity: http://www.geeksforgeeks.org/g-fact-86/
 */
public static int fibo(int n) { … }

A naïve recursive fibo() has O(2n) time complexity. Think of it this way: to calculate fibo(n), you break it up into two problems, each of size n - 1.

In fiboBottomUp(), don't declare/define int a = 0, since it is only ever used as a temporary variable inside the for-loop.

public static int fiboBottomUp(int n) {
    if (n < 0) {
        // Message is inaccurate: n = 0 is allowable
        throw new IllegalArgumentException("The value of n: " + n  + " should be non-negative.");
    }

    int b = 1;
    int c = 0;
    for (int i = 0; i < n; i++) {
        int a = b;
        b = c;
        c = a + b;
    }
    return c;
}

Using a HashMap<Integer, Integer> for fiboCache is more complicated than necessary. An ArrayList<Integer> or even an int[n + 1] will do, since all the keys are consecutive integers.

Consider widening your return types to long to stave off overflow for a while longer.

Source Link
200_success
  • 144.2k
  • 22
  • 188
  • 473

This comment is inaccurate:

/*
 * TimeComplexity: O(n)
 * Space Complexity: http://www.geeksforgeeks.org/g-fact-86/
 */
public static int fibo(int n) { … }

A naïve recursive fibo() has O(2n) time complexity. Think of it this way: to calculate fibo(n), you break it up into two problems, each of size n - 1.

Using a HashMap<Integer, Integer> for fiboCache is more complicated than necessary. An ArrayList<Integer> or even an int[n + 1] will do, since all the keys are consecutive integers.

Consider widening your return types to long to stave off overflow for a while longer.