Resolve code duplication using Generics VS Inheritance

36 views Asked by At

I have a common reader for three different classes, but there is some code duplication because the reader has different methods for each of the classes A, B, and C. Is there anyway I can make the code cleaner? e.g., by having only one read method?

Any insights would be appreciated :)

(Due to legacy issues I wasn't able to let A, B, and C extend another superclass or implement another superclass.)

public class ReaderForABC {

    public A readA() {
        doLotsOfCommonThings();
        A result = computeA();
        return result;
    }
    public B readB() {
        doLotsOfCommonThings();
        B result = computeB();
        return result;
    }
    public C readC() {
        doLotsOfCommonThings();
        C result = computeC();
        return result;
    }
    
    private static void doLotsOfCommonThings(){}
    private static A computeA(){
        //do something common
        //call helper function to compute A
        /*This helper function is similar to computeA(), 
        where there are some code duplication and some differences in the end
        (and also a cascade of calls to similar functions like computeA())
        */
         
    }
    private static B computeB(){
        //do something common
        //call helper function to compute B
        /*This helper function is similar to computeB(), 
        where there are some code duplication and some differences in the end
        (and also a cascade of calls to similar functions like computeB())
        */
    }
    private static C computeC(){
        //do something common
        ///call helper function to compute B
        /*This helper function is similar to computeC(), 
        where there are some code duplication and some differences in the end
        (and also a cascade of calls to similar functions like computeC())
        */
    }
}

To use the reader:

ReaderForABC readerForABC = new ReaderForABC()
A a = readerForABC.readA();
A b = readerForABC.readB();
//...similarly for C.

The main issue I face is that those readX and computeX functions have lots of code duplication because computeX calls a cascade of other functions that have code duplication for each type of A, B, and C. If I have a way to address code duplication in readX, I can address similarly in computeX and other functions that are called within it.

I tried to use generics, but it made things worse...because it has to decide on which action to take by checking class type

public <T> T read(Class<T> classType) {
        doLotsOfCommonThings();
        T result;
        if (classType.getSimpleName().equals("A")) {
            result = (T) computeA();
            return result;
        } else if (classType.getSimpleName().equals("B")) {
            result = (T) computeB();
            return result;
        }
        //...handle C similarly
    }

(edited computeX() to make things more clear, hopefully)

1

There are 1 answers

0
tgdavies On

The classic solution would be to have a class hierarchy with an abstract base class and a concrete class for each of A, B and C:

public abstract class AbstractReader<T> {
  protected void doLotsOfCommmonThings() {
    // do the things
  }
  protected abstract T compute();
  public T read() {
    doLotsOfCommmonThings();
    return compute();
  }
}

public class AReader extends AbstractReader<A> {
   protected A compute() {
     // do the computing
   }
}

public class BReader extends AbstractReader<B> {
   protected B compute() {
     // do the computing
   }
}

...

Duplicate code in the compute methods can also be moved into protected methods of AbstractReader.

The other way to do it, which I would probably choose, uses composition:

public class ThingDoer {
  private CommonThingsResult doLotsOfCommmonThings(CommonThingsParameters params) {
    // do the things
  }

  public <T> T read(Function<CommonThingsResult, T> computeFn) {
    return computeFn.apply(thingDoer.doLotsOfCommmonThings(...));
  }
}

public class AReader {
  private final ThingDoer thingDoer;

  public AReader(ThingDoer thingDoer) {
    this.thingDoer = thingDoer;
  }

  public A read() {
    return thingDoer.read(AReader::compute);
  }

  private A compute(CommonThingsResult result) {
   // compute an A
  }
}

Which allows testing ThingDoer separately from A/B/CReader

You can put other public methods on ThingDoer to call in your compute implementations, or have another class for them.