Resolve code duplication using Generics VS Inheritance

Question

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)

What does each of the methods do? If they all return different types, then they probably do different things, and are therefore not code duplication. Can you show the implementations of those methods? — Sweeper
– Sweeper, Commented Jun 8, 2023 at 2:44
If A,B and C shared a common interface, then you could reduce this is one method, which would take a a parameter indicating which computer method to call — Scary Wombat
– Scary Wombat, Commented Jun 8, 2023 at 2:47
Are you reading A, B and C or are you computing A, B and C? Are A, B and C related through a type hierarchy? Can they be? Perhaps a Computable interface? What are you doing with the results? — Elliott Frisch
– Elliott Frisch, Commented Jun 8, 2023 at 2:47
What does the code where you call the readX methods look like? — tgdavies
– tgdavies, Commented Jun 8, 2023 at 2:49
I should've also mentioned that computeA(), computeB() and computeC() are also very similar (but complex) functions that can be improved via code reuse. Getting an idea on how to clean up the read() function would help me reduce code duplications in these functions too — KII
– KII, Commented Jun 8, 2023 at 2:49

tgdavies · Accepted Answer · 2023-06-08 04:00:33Z

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.

Collectives™ on Stack Overflow

Resolve code duplication using Generics VS Inheritance

1 Answer 1

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Related