0

I have one interface with let's say 2 methods and has 3 different implementations.

public interface IFace {
    public void method1(Param1 param1);
    public void method2(Param2 param2);
}

public class A implements IFace {
    public void method1(Param1 param1){}
    public void method2(Param2 param2){}
}

public class B implements IFace {
    public void method1(Param1 param1){}
    public void method2(Param2 param2){}
}

public class C implements IFace {
    public void method1(Param1 param1){}
    public void method2(Param2 param2){}
}

Requirement: one of two methods needs modifications. Pre-conditions decide which specific implementation to call.

I need a pattern to go through those implementations one by one. This appeared like a perfect fit for chain of responsibility and I created that like this:

public interface IChain {
    public void method1(Param1 param1);
    public void setNextChain(IChain iChain);
} 

public class AA implements IChain {
    private IChain chain;

    private IFace a;

    public void method1(Param1 param1){
        if (thisConditionIsSatisfied(param1)) {
            a.method1(param1);
        } else {
            chain.method1();
        }
    }

    public void setNextChain(IChain chain){
        this.chain = chain
    }

    public void setA(IFace a) {
        this.a = a;
    }
}

public class BB implements IChain {
    private IChain chain;

    private IFace b;

    public void method1(Param1 param1){
        if (thisConditionIsSatisfied(param1)) {
            b.method1(param1);
        } else {
            chain.method1();
        }
    }

    public void setNextChain(IChain chain){
        this.chain = chain
    }

    public void setA(IFace b) {
        this.b = b;
    }
}

public class CC implements IChain {
    private IChain chain;

    public void method1(Param1 param1){
        if (thisConditionIsSatisfied(param1)) {
            //process it here
        } else {
            throw new RuntimeException("Couldn't process request.")
        }
    }

    public void setNextChain(IChain chain){
        this.chain = chain
    }

}

As you can see, CC doesn't delegate to class C which was implementing IFace and is a completely new implementation.

This is a very small part that I am displaying but the problem effectively is the same that I see a lot of duplication in terms of condition evaluation, exception handling, calling the correct implementation of the interface etc.

Though this is much better than trying to extend the existing interface but I was wondering if there is any recommendation to make it better in terms of future extensibility or OOPS pattern following.

  • 1
    Following (design) patterns is not a goal. – Derek Elkins Aug 12 '17 at 0:16
  • The I for interface prefix thing you use in IFace is a c# thing not a Java thing and certainly not a good thing. Please limit its use to c#. – candied_orange Aug 12 '17 at 1:58
  • An example of how you intend to use this code would help. – candied_orange Aug 12 '17 at 3:06
  • The Chain of Responsibility pattern is not necessarily the best choice here; it's good when you already have a structure that is chain-like or hierarchical. (E.g., consider custom GUI controls - a window contains a panel, which contains a button. The user clicks the button, but its the window that receives the mouse event, and then forwards it down to the descendant controls until one of them handles it.) But here, it looks like you are imposing the chain structure artificially. Maybe you can simply use a Factory to create an instance of a concrete type depending on your conditions? – Filip Milovanović Aug 12 '17 at 14:31
  • Have you looked into using the Visitor Pattern instead? – ZeroOne Aug 20 '17 at 22:52
1

Other than the change in the name of a private variable (a to b) AA is exactly the same as BB. CC is the same except for a pointless hardcoding of an exception that could have been done by changing the implementation of method1() on the chain member. Also if method2() isn't used by anything get rid of it.

You have a lot of duplication because you've created it pointlessly. AA does everything you need. Give it a better name and use it. Get rid of BB and CC.

I was wondering if there is any recommendation to make it better in terms of future extensibility or OOPS pattern following

I recommend this change:

public class AA implements IChain {
    private IChain chain;

    private IFace a;

    Condition condition; //New code

    public void method1(Param param1){
        if (condition.thisConditionIsSatisfied(param1)) { //New code
            a.method1(param1);
        } else {
            chain.method1();
        }
    }

    public void setNextChain(IChain chain){
        this.chain = chain
    }

    public void setA(IFace a) {
        this.a = a;
    }

    public void setCondition(Condition condition) { //New code
        this.condition = condition; //New code
    }
}

This way the ONLY code implementation in this terribly named AA class is the chain of responsibility pattern. What it's checking, doing, or chaining is all something else's problem.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.