Wsrtjtyk

I'm trying to master the refactoring for a project I'm working on right now. Basically I have two classes which each extend from an interface.

If you take a look at my code, you can observe that there is a lot of code duplication, as the methods' implementations across each class are almost exactly the same - they just use different object types (Though InvestmentRequests and FundingRequests also both implement the same interface).

So what would be the ideal approach for refactoring this code? Can it be done from the interface level? I've tried to do it by declaring the objects in my interface like so:

RequestsData allRequests = null;

RequestsData fixedRequests = null;

RequestsData trackerRequests = null;

but that doesn't look like what I'm trying to do, and I'm unsure of the syntax.

Interface

public interface RequestDataBase<E,T> {



    T getAllRequests();



    T getFixedRequests();



    T getTrackerRequests();



    void add(E newRequest);



    void addAll(List<E> accounts);

}

Class A

public class FundingRequestData implements RequestDataBase<FundingRequest,FundingRequestData.FundingRequests> {



private static FundingRequests fundingRequests;

private static FundingRequests fixedFundingRequests;

private static FundingRequests trackerFundingRequests;



private static FundingRequestData instance = new FundingRequestData();



public static FundingRequestData getInstance() {

    return instance;

}



private FundingRequestData() {

    fundingRequests = new FundingRequests();

    fixedFundingRequests = new FundingRequests();

    trackerFundingRequests = new FundingRequests();

}



@Override

public FundingRequests getAllRequests() {

    return fundingRequests;

}



@Override

public FundingRequests getFixedRequests() {

    return fixedFundingRequests;

}



@Override

public FundingRequests getTrackerRequests() {

    return trackerFundingRequests;

}



private void listSpecifier(FundingRequest request) {

    if (request.getType().equals("FIXED")) {

        fixedFundingRequests.add(request);

    } else {

        trackerFundingRequests.add(request);

    }

}



@Override

public void add(FundingRequest newRequest) {

    fundingRequests.add(newRequest);

    listSpecifier(newRequest);

}



@Override

public void addAll(List<FundingRequest> accounts) {

    fundingRequests.getRequests().addAll(accounts);

    for (FundingRequest request : accounts) {

        listSpecifier(request);

    }

}

Class B

public class InvestmentRequestData implements RequestDataBase<InvestmentRequest,InvestmentRequestData.InvestmentRequests> {

    private static InvestmentRequests investmentRequests;

    private static InvestmentRequests fixedInvestmentRequests;

    private static InvestmentRequests trackerInvestmentRequests;



    private static InvestmentRequestData instance = new InvestmentRequestData();



    public static InvestmentRequestData getInstance() { return instance; }



    private InvestmentRequestData() {

        investmentRequests = new InvestmentRequests();

        fixedInvestmentRequests = new InvestmentRequests();

        trackerInvestmentRequests = new InvestmentRequests();

    }



    public void investAll() {

        for (InvestmentRequest request : investmentRequests.getUnfulfilledRequests()) {

            request.investAll();

        }

    }



    public InvestmentRequests getAllRequests() {

        return investmentRequests;

    }



    public InvestmentRequests getFixedRequests() { return fixedInvestmentRequests; }



    public InvestmentRequests getTrackerRequests() {

        return trackerInvestmentRequests;

    }



    private void listSpecifier(InvestmentRequest newRequest) {

        if(newRequest.getType().equals("FIXED")) {

            fixedInvestmentRequests.add(newRequest);

        } else {

            trackerInvestmentRequests.add(newRequest);

        }

    }



    public void add(InvestmentRequest newRequest) {

        investmentRequests.add(newRequest);



        listSpecifier(newRequest);

    }



    public void addAll(List<InvestmentRequest> newRequests) {

        for (InvestmentRequest request : newRequests) {

            listSpecifier(request);

        }

    }

You won't be able to refractor the generic objects on the interface level otherwise you would be refractoring both FundingRequest(s) and InvestmentRequest(s) to the same object, which I don't think is what you intend.

I would refractor the objects within class A and B. However, the general design can probably be improved. Something like having InvestmentRequest and FundingRequest implementing a Request interface, so instead of using generics E and T in the RequestDataBase interface, you can use Request and Requests objects.

If FundingRequest and InvestmentRequest both implement the same interface (Request) then you should only ever deal with Requests.

If you code your class to only interact with Requests, I'm guessing you encounter some cases where you have to treat the two types differently (otherwise you'd already be done!)

If you have to treat them differently, I suggest a two-step refactor (Run unit tests between each step!)

First use if(x instanceof FundingRequest) to choose one code path or the other. This lets you make a minimum refactor that should work. The goal of this part is to condense the two classes you are discussing into a single class.

Do not stop there though, After you get it all refactored like that, now you need to push those instanceofs into the two request classes. Possibly add a new method to the interface that calls an implementation and put one path of that if() statement into FundingRequest and the other into InvestmentRequest.

When you are done with this part of the refactor your class should only refer to "Requests", never FundingRequest or InvestmentRequest.