2

Supposed I have a common block of code between two controllers where the only difference is the concrete class that the block uses. Both clases implemenet a common interface so I am using an interface to interact with the object.

The block of code is a series of steps that interact with functions of the concrete class. The class wraps a 3rd party library and handles errors and re-trys since the 3rd party library calls a remote api.

I believe these blocks DRY principal so I am looking to refactor them out of the controller. My first thought is to add another function to the interface and move the block to the class instance. However this is still in violation of DRY.

Another option is to move it to a utility class and run through the block there but that does not seem like the "clean" way build this.

I know this block does not belong in the controller if it has to duplicate the same block with a small difference. Should this be moved to the busieness logic layer? Or is there a design pattern that can help solve this instead of moving this to a utility class?

Example Added Suppose I have two controllers which perform the same sequence of steps but only differ by the concrete class that is needed.:

class Controller1 {
    IWrapper wrapper = Wrapper1();
    public ActionResult DoSequence(){
        wrapper.SetepOne();
        wrapper.StepTwo();
        wrapper.StepThree();
        return View();
    }
}

class Controller2 {
    IWrapper wrapper = Wrapper2();
    public ActionResult DoSequence(){
        wrapper.SetepOne();
        wrapper.StepTwo();
        wrapper.StepThree();
        return View();
    }
}

I want to move the block of code,

wrapper.SetepOne();
wrapper.StepTwo();
wrapper.StepThree();

out of the controllers so it would be something like this,

class Controller1 {
    IWrapper wrapper = Wrapper1();
    public ActionResult DoSequence(){
        <some class>.RunSequence(wrapper);
        return View();
    }
}

class Controller2 {
    IWrapper wrapper = Wrapper2();
    public ActionResult DoSequence(){
        <some class>.RunSequence(wrapper);
        return View();
    }
}

class <Some Class>{
    public void RunSequence(IWrapper wrapper)
    {
        wrapper.SetepOne();
        wrapper.StepTwo();
        wrapper.StepThree();
    }
}

I am wondering if there is a design pattern that would help to build out <Some Class> or an architectural pattern that would help decide where that class should go.

  • A simple example might help this... – FrustratedWithFormsDesigner Dec 19 '17 at 15:25
  • @FrustratedWithFormsDesigner does this example help? – DFord Dec 19 '17 at 15:45
  • Yes, that makes it a bit clearer what you are talking about. A bit busy now, will try to come up with a better answer later if there's time... – FrustratedWithFormsDesigner Dec 19 '17 at 15:57
  • Would it be possible to create a base Controller class, i.e. WrapperController (perhaps abstract) with the common code? The possible drawback is that if the controller hierarchy gets deep and complicated and parallels a complex Wrapper class hierarchy. A little bit of inheritance is o.k., but if messy prefer composition. – user949300 Dec 19 '17 at 16:31
  • My specific case is a .NET MVC project so the Controller inherits from the .NET Controller base class. – DFord Dec 19 '17 at 16:42
5

The classic design pattern for this is the template method pattern. This would lead to a design like

class AbstractController{
    IWrapper wrapper;
    public ActionResult DoSequence() {
        wrapper = CreateWrapper();
        wrapper.StepOne();
        wrapper.StepTwo();
        wrapper.StepThree();
        return View();
    }
    protected abstract IWrapper CreateWrapper();
}

Now you can derive Controller1 and Controller2 from AbstractController and implement CreateWrapper in each derived class accordingly.

However, in this case you could also use generics (in Java or C#, or templates in C++) for solving your problem:

class Controller<T>  {
    IWrapper wrapper = T();
    public ActionResult DoSequence(){
         wrapper.StepOne();
         wrapper.StepTwo();
         wrapper.StepThree();
         return View();
    }
}

(exercise for the reader: correct the syntax to your programming language).

This is shorter since there is no need to derive anything here, however, the users of that class now must be aware of what type they can pass for the parameter T.

  • 1
    +1; you could also use the generics approach, and create the specific controllers by doing: class Controller2 : Controller<Wrapper2> { } – Emerson Cardoso Dec 19 '17 at 18:48
  • This Q doesn't specify language and I am not 100% on C#, but you could also limit what T can be (at least in Java), that way users will be limited to what they can pass as T. Something like: class Controller<T extends Wrapper> – Hangman4358 Dec 20 '17 at 14:13
  • @Hangman4358: see my edit. Syntactical details are not my focus. – Doc Brown Dec 20 '17 at 14:16
2

I see two ways to go here:
At first sight I think about abstraction and inheritance, reuse common business logic using a super class, you can move the block of code to an abstraction and then inherit it in both controllers without exposing it to others (favors encapsulation).

class AbstractController {
    protected void runSequence() {
        wrapper.SetepOne();
        wrapper.StepTwo();
        wrapper.StepThree();
    }
}

class Controller1 extends AbstractController {
    IWrapper wrapper = Wrapper1();
    public ActionResult DoSequence(){
        this.RunSequence(wrapper);
        return View();
    }
}

class Controller2 extends AbstractController {
    IWrapper wrapper = Wrapper2();
    public ActionResult DoSequence(){
        this.RunSequence(wrapper);
        return View();
    }
}

You can also create a different class to externalize the business logic, this is called Visitor Pattern, may be it's a better fit in situations where the business logic is the same in other places too (favors reuse).

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.