How can I remove duplicated code between classes without implements?

Question

I wrote many implementations of a function to compute the Fibonacci number at a given position.

Fibonacci Class: this class helps me to test each implementation without rewrite same test code. I don't want to add "fibonacci(n - 2).add(fibonacci(n - 1));" here because some implementations do not use it (imperative iteration, functional iteration).

public interface Fibonacci {
  BigInteger fibonacci(int n);

}

RecursiveFibonacci Class

public class SimpleRecursiveFibonacci implements Fibonacci{

  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 implements Fibonacci{
  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?

Make static factory method. + `Fibonacci` is not a class but an interface. — Antoniossss, Oct 31 '18 at 19:50

score 2 · Accepted Answer · answered Oct 31 '18 at 19:51

2

The MemorizedRecursiveFibonacci can delegate to a RecursiveFibonacci instance:

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

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

    return cache.get(n);
  }
}

Or, even more elegantly, using Java 8's Map#computeIfAbsent:

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

  public BigInteger fibonacci(int n) {
    return cache.computeIfAbsent(n, k -> simple.fibonacci(k));
}

answered Oct 31 '18 at 19:51

Mureinik

297,002
52
306
350

2

Your answers are right, but it made memoized version is slower than the recursive version. – Minh Ngo Oct 31 '18 at 21:23
@MinhQuangNgo Why would the memoized version be slower? The cache is being used so already calculated values doesn't get calculated again. – Progman Oct 31 '18 at 23:48
2

@Mureinik SimpleRecursiveFibonacci does not take advantage of the cache. If `n` is not in the cache, calling the memoized function is the same as calling the simple one, even if `n-1` and `n-2` are in the cache. One reason it could be slower is from the overhead of the searching the cache on cache misses. – eclipz905 Nov 01 '18 at 13:49
1

This implementation would only work if the cache was pre-populated - using e.g. `var memoized = new MemoizedRecursiveFibonacci(); memoized(100);` (which would take *ages* to calculate. From there on, any calls to `memoized.fibonacci(n)` for n < 100 would be quick. But that's not the definition of "memoized". A proper memoized method should *use* previously calculated values; therefore it cannot do it by delegating to the simple recursive version. Also, `Map.computeIfAbsent` cannot be used to implement a memoized fibonacci method. – DodgyCodeException Nov 01 '18 at 17:37

score 1 · Answer 2 · answered Oct 31 '18 at 19:59

What about an abstract common parent? Something like this:

public abstract class ParentFibonacci implements Fibonacci {
    protected BigInteger getFirstValues(int n) {
        if (n < 2) {
            return BigInteger.ONE;
        }
        return BigInteger.ZERO;
    }
}

This way your Fibonacci implementation need to implement Fibonacci.fibonacci(int n) and can use parent methods.

public class SimpleRecursiveFibonacci extends ParentFibonacci {

    public BigInteger fibonacci(int n) {
        if (n < 2) {
            return getFirstValues();
        }
        return fibonacci(n - 2).add(fibonacci(n - 1));
    }
}

How can I remove duplicated code between classes without implements?

2 Answers2