How can I remove duplicated code between classes?

Question

I have 2 class: RecursiveFibonacci and MemorizedRecursiveFibonacci. This is what I have so far.

RecursiveFibonacci Class

public class SimpleRecursiveFibonacci {

  public BigInteger fibonacci(int n) {
    if(n < 2) {
      return BigInteger.ONE;             
    }

    return fibonacci(n - 2).add(fibonacci(n - 1));
  }
}

and MemorizedRecursiveFibonacci Class

public class MemoizedRecursiveFibonacci {
  private Map<Integer, BigInteger> cache = new HashMap<>();

  public BigInteger fibonacci(int n) {
    if(n < 2) {
      return BigInteger.ONE;
    }
    if(!cache.containsKey(n)){
      BigInteger currentFibonacci = fibonacci(n - 2).add(fibonacci(n - 1));
      cache.put(n, currentFibonacci);
    }

    return cache.get(n);
  }
}

As I see, there are some duplicated code in MemorizedRecursiveFibonacci Class

 if(n < 2) {
      return BigInteger.ONE;

and

  BigInteger currentFibonacci = fibonacci(n - 2).add(fibonacci(n - 1));

How can I keep it DRY? remove duplicated code?

What about using MemoizedRecursiveFibonacci and implement a method like public BigInteger fibonacci(int n, bool useCache) and lets the caller to use the cache or not? Or using an attribute @Cacheable to decorate the method to mantain a cache? — user2896152, Oct 31 '18 at 17:40
An unrelated observation: the memoized version always populates the HashMap with consecutive Integer keys. Therefore it would be more efficient to use an ArrayList and use the index instead of the key. — DodgyCodeException, Nov 01 '18 at 15:09

score 6 · Answer 1 · answered Oct 31 '18 at 17:46

6

Maybe this is an option... but not the best i think.

public class SimpleRecursiveFibonacci {

public BigInteger fibonacci(int n) {
    if(n < 2) {
        return BigInteger.ONE;
    }
    return calculate(n);
}

protected BigInteger calculate(int n){
    return fibonacci(n - 2).add(fibonacci(n - 1)),
}

}

public class MemoizedRecursiveFibonacci extends SimpleRecursiveFibonacci{
private Map<Integer, BigInteger> cache = new HashMap<>();

@Override
protected BigInteger calculate(int n) {
    if(!cache.containsKey(n)){
        BigInteger currentFibonacci = super.calculate(n);
        cache.put(n, currentFibonacci);
    }
    return cache.get(n)
}

}

answered Oct 31 '18 at 17:46

pL4Gu33

2,045
16
38

1

It's not bad actually. Classic template method :) Only improvement I'd make is to return `currentFibonacci` early instead of looking it up again. – StoryTeller - Unslander Monica Oct 31 '18 at 17:58
1

Similar to what I came up with. I use a third method for how to "get" the fibonacci value, either from a cache or by recalculating it. – xtratic Oct 31 '18 at 17:59

xtratic · Accepted Answer · 2019-01-10T20:10:43.407

How about something like this:

public class SimpleRecursiveFibonacci {

    /** Gets the fibonacci value for n */
    public final BigInteger fibonacci(int n) {
        if (n == 0) {
            return BigInteger.ZERO;
        } else if (n == 1) {
            return BigInteger.ONE;
        }
        return getFibonacci(n);
    }

    /** Recursively calculates the fibonacci by adding the two previous fibonacci. */
    protected final BigInteger calculateFibbonacci(int n) {
        return fibonacci(n - 2).add(fibonacci(n - 1));
    }

    /** 
     * Somehow get the fibonacci value for n.
     * Could be by calculation, getting it from a cache, or anything.
     * Defaults to calculation.
     */
    protected BigInteger getFibonacci(int n) {
        return calculateFibbonacci(n);
    }

}

public class MemoizedRecursiveFibonacci extends SimpleRecursiveFibonacci {

    // Cache using an array list as recommended by user @DodgyCodeException
    private ArrayList<BigInteger> cache = new ArrayList<>();

    @Override
    protected BigInteger getFibonacci(int n) {
        if (cache.size() < n) {
            BigInteger fib = calculateFibbonacci(n);
            cache.add(fib);
            return fib;
        } else {
            return cache.get(n - 1);
        }
    }
}

score 3 · Answer 3 · answered Nov 01 '18 at 11:47

Another example using Java 8 features - BiFunction interface with lambda expression:

BiFunction<Fibonacci, Integer, BigInteger> func = (fibonacci, n) -> {
    if (n < 2) {
        return BigInteger.ONE;
    }
    return fibonacci.calc(n - 2).add(fibonacci.calc(n - 1));
};

new CachedFibonacci(func).calc(100);

Implementation:

interface Fibonacci {
    BigInteger calc(int n);
}

class SimpleFibonacci implements Fibonacci {

    private BiFunction<Fibonacci, Integer, BigInteger> fibonacci;

    SimpleFibonacci(BiFunction<Fibonacci, Integer, BigInteger> fibonacci) {
        this.fibonacci = fibonacci;
    }

    public BigInteger calc(int n) {
        return fibonacci.apply(this, n);
    }
}

class CachedFibonacci implements Fibonacci {

    private BiFunction<Fibonacci, Integer, BigInteger> fibonacci;
    private Map<Integer, BigInteger> cache = new HashMap<>();

    CachedFibonacci(BiFunction<Fibonacci, Integer, BigInteger> fibonacci) {
        this.fibonacci = fibonacci;
    }

    public BigInteger calc(int n) {
        if (!cache.containsKey(n)) {
            cache.put(n, fibonacci.apply(this, n));
        }
        return cache.get(n);
    }
}

This works, it does what it is supposed to do, but I find it a bit hard to follow. It seems to trade reduced readability for removing duplication. — DodgyCodeException, Nov 01 '18 at 15:06

score -2 · Answer 4 · answered Oct 31 '18 at 17:24

use static method:

public class SimpleRecursiveFibonacci {

  public static BigInteger fibonacci(int n) {
    if(n < 2) {
      return BigInteger.ONE;             
    }

    return fibonacci(n - 2).add(fibonacci(n - 1));
  }
}

public class MemoizedRecursiveFibonacci {
  private Map<Integer, BigInteger> cache = new HashMap<>();

  public BigInteger fibonacci(int n) {
    if(!cache.containsKey(n)){
      BigInteger currentFibonacci = SimpleRecursiveFibonacci.fibonacci(n);
      cache.put(n, currentFibonacci);
    }
    return cache.get(n);
  }
}

This will only memoize the final value, and not intermediate ones. — StoryTeller - Unslander Monica, Oct 31 '18 at 17:26

How can I remove duplicated code between classes?

4 Answers4