0

I'm currently implementing functionality in Symfony (PHP - not that it should make any difference which language I'm using) where I have about four controllers that do almost exactly the same thing. The only difference between them is that they use different entity objects and form objects.

I've already refactored all the common functionality into an abstract parent class. I then have two abstract methods called getEntityInstance and getFormInstance, which only return the appropriate object instances.

The entity and form instances returned would both have common interfaces, called AbstractQuery and AbstractForm respectively.

As an example, let's say I have the following two classes.

abstract class AbstractQueryController {
    abstract protected function getEntityInstance();

    abstract protected function getFormInstance(AbstractQuery $query);

    public function queryAction(Request $request) {
        //Do some stuff
        $form = $this->getFormInstance($this->getEntityInstance());
        //Do something with $form
    }
}

class SomeRandomController extends AbstractQueryController() {
    protected function getEntityInstance() {
        return new SomeRandomQuery();
    } 

    protected function getFormInstance(AbstractQuery $query) {
        return new SomeRandomForm($query);
    }
}

Now this approach works, but it looks wrong. What design pattern or other concept should be applied here to make the code "cleaner"? The issue is that, with every new controller that extends AbstractQueryController, I'm basically baking the "configuration" (if you can call it that) into the child class, which looks incorrect.

  • What you're doing is a great example of the Template Method design pattern, which is often used as a Strategy. You are instead using it to inject dependencies. That is totally OK, but I would make the DI more explicit by using constructor injection, and a separate factory to manage object creation. That way, you can do away with most subclassing. – amon Oct 25 '15 at 11:47
  • I can't wait until the day where I will be certain that this kind of meta-meta OO design noodling is completely unnecessary for the rest of my career in software development. It makes me want to switch to Haskell today. Obtuse classes for "controllers" aren't bad enough, so we add abstract classes for them? What ever happened to the original point of OOP of "making code more readable" by "representing real-life things"? – John Doe Dec 24 '15 at 21:37
1

It depends on what you want to do exactly, which is a bit unclear from your example.

However there are certain issues with your design, such as the Controller being directly responsible for creating needed objects. This leads to very tight coupling, very hard to test code and other issues.

I would begin refactoring your example by moving the creation of Query and Form instances into factories and then inject these factories into your controller.

You can have different factories, each producing a different instance of Query and Form classes (note that these classes should implement common interface, QueryInterface and/or FormInterface) and create an IoC container which will be responsible to injecting the correct factory into your controller (or concrete method) based on user input - be it url parsing or anything else.

If the current logic for hangling the result of the getFormInstance method is always the same, correctly implementing factories and the IoC container acting as strategy pattern, you could have as little as only one Controller handling all your request and would not need to have more of them.

The return type of getFormInstance would then be an interface which specific Forms would implement and using the interface instead of specific classes would guarantee you the behaviour by the interface's definition.

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.