4

I have a system that allows certain functionality to be implemented different ways, but requires that the functionality of each implementing class be wrapped in another layer. For example, each implementor has a doFoo method that interacts with an API. The implementor classes can be loaded at runtime (they are, essentially, plugins for hooking into different APIs), and thus the set of possible implementors is not fixed and cannot simply be handled by a switch or an enum-like structure.

But each time one of the implementor classes executes its doFoo method, I also need to ensure that additional, method-specific code occurs before and after (think system-wide logging, error-handling, and input and output sanity-checking that are the same for all implementors).

I have the following basic setup:

  • an interface, FooInterface
  • an abstract class FooAbstract that implements FooInterface and sets up some default actions.
  • a concrete class Foo that extends FooAbstract. Foo serves as a kind of factory/bridge class that instantiates an implementor and adds its own code when calling any of the implementor's methods
  • concrete classes FooImplementorABC, FooImplementorXYZ, etc., each of which extends FooAbstract.

That way, when a user wants to use the ABC implementation of FooInterface, the user calls new Foo('ABC'). The Foo constructor instantiates the right implementor and the implementor's code as necessary.

Example code (sample code only; the real classes are more complex and are all broken into files and autoloaded appropriately):

<?php
interface FooInterface
{
    public function doFoo();
    public function doBar();
    public function doGenericThing();
}

abstract class FooAbstract implements FooInterface
{
    abstract public function doFoo();
    abstract public function doBar();

    public function doGenericThing()
    {
        echo "Doing something generic\n";
    }
}

class Foo extends FooAbstract
{
    /** @var FooAbstract */
    private $implementor;

    /**
     * The constructor for this class accepts an implementation name.
     * It then attempts to instantiate the appropriate implementor
     * class.
     *
     * @param $implementationName String
     */
    public function __construct($implementationName)
    {
        $implementorClassName = 'FooImplementor' . $implementationName;
        try {
            if (class_exists($implementorClassName)) {
                $this->implementor = new $implementorClassName(); // Yes, this works in PHP! One of its best or worst features, depending on your perspective.
            } else {
                throw new Exception("ERROR: No implementation for $implementationName found!");
            }
        } catch (Exception $e) {
            echo "\n\n{$e->getMessage()}\n\n";
        }
    }

    public function doFoo()
    {
        echo "Doing Foo\n";
        // Do some stuff
        $this->implementor->doFoo();
        // Do some stuff
        echo "Done with Foo\n\n";
    }

    public function doBar()
    {
        echo "Doing Bar\n";
        // Do some stuff
        $this->implementor->doBar();
        // Do some stuff
        echo "Done with Bar\n\n";
    }
}

class FooImplementorABC extends FooAbstract
{
    public function doFoo()
    {
        echo "Foo Implementor ABC just did Foo!\n";
    }

    public function doBar()
    {
        echo "Foo Implementor ABC just did Bar!\n";
    }
}

class FooImplementorXYZ extends FooAbstract
{
    public function doFoo()
    {
        echo "Foo Implementor XYZ just did Foo!\n";
    }

    public function doBar()
    {
        echo "Foo Implementor XYZ just did Bar!\n";
    }
}

$myFooABC = new Foo('ABC');
$myFooXYZ = new Foo('XYZ');

$myFooABC->doFoo();
$myFooXYZ->doFoo();
$myFooABC->doBar();
$myFooXYZ->doBar();

$myFooQRS = new Foo('QRS'); // displays an error message

All of this works just fine; the methods all get wrapped correctly, the implementation details are unimportant to the consumer, etc.

My questions are:

  1. Is this a pattern or an anti-pattern?
  2. If it's a pattern, is there a name for it? I can't figure out what it would be. It seems to me like a hybrid of the factory, bridge, and decorator patterns.
  3. If it's an anti-pattern, why? How is this going to bite me down the road?
  • Question aside: I don't get why you throw an exception an immediately catch it for printing. Wouldn't have been clearer to directly print the message in the else clause ? (thus, removing the try...catch structure) – Spotted Oct 16 '15 at 13:51
  • @Spotted This is just example code. In the real world, I would in fact treat this as an exception, log it, and show an appropriate message to the end user. – Ed Cottrell Oct 16 '15 at 13:53
3

Foo seems to be both a factory and a decorator(*). By the Single Responsibility Principle that could be an anti-pattern and you may want to split it (e.g. what if some day you want an implementation of FooInterface but don't want Foo's additions?)

Moreover I don't see what's the abstract class for. Maybe in PHP you can't implement directly an interface?

(*) many patterns look similar and differ only by intent. I'm not too good at distinguishing them, but I don't see a bridge.

  • I'm not sure how to split Foo. Can you provide any insight? Would I use a factory to create the appropriate implementor, then pass the implementor to a decorator to add my wrappers? As for your second point: the abstract class is kind of an intermediary to get around your first point. In my real code, the abstract class has several non-abstract functions. So, you could extend FooAbstract to take advantage of the default functionality in the abstract class. But yes, you could implement the interface directly. – Ed Cottrell Oct 15 '15 at 20:37
  • @EdCottrell That's what I had in mind, or you could pass to the wrapper the whole factory already setup (i.e. built with the correct impl. name) and call create() only in doFoo/Bar(). That could depend on if you want to defer instantiating the implementor for resources concerns. – bigstones Oct 15 '15 at 20:52
1

In object-oriented programming, this would probably have the closest resemblance to the Strategy pattern due to subclassing with the same semantics but with potentially different algorithms.

the strategy pattern (also known as the policy pattern) is a software design pattern that enables an algorithm's behavior to be selected at runtime.

There are some elements of the Decorator pattern in that you are wrapping the chosen strategy algorithm's call with some code on the base class.

the decorator pattern (also known as Wrapper, an alternative naming shared with the Adapter pattern) is a design pattern that allows behavior to be added to an individual object, either statically or dynamically, without affecting the behavior of other objects from the same class.

The implementation of the decorator pattern is not ideal, since you can only attach one decorator... the one that's wired into the base class. But if that's all you need, fine.

The combination of patterns might be more effectively achieved another way (although I'm unsure which oo constructs PHP has available). For instance, an interface is typically used for the strategy pattern so the same methods can be called no matter the actual object underneath. The decorator could be implemented a number of ways, but one way is to have a wrapper class which takes both an interface for the strategy, and the decorators which wrap the call to the interface.

  • This is really helpful; thanks. I may need clarification after I digest this a bit more. – Ed Cottrell Oct 15 '15 at 20:41
  • Sure thing. :-) – Kasey Speakman Oct 15 '15 at 20:42
0
  1. an anti-pattern
  2. N/A
  3. because you rely on a string to instanciate $implementor. Imagine you refactor the class' names from FooImplementorABC and FooImplementorXYZ to MyClass and BarWXY. Your "factory" (Foo) won't be able to instanciate any implementor and you have no way to figure it out except by diving into the code. Plus the class Foo has 2 responsibilities (which violates the SRP): finding the correct implementation of FooAbstract and echoing some things.

My suggestion: delegate the responsibility for the FooImplementor creation outside of the class.

class LoggingFoo extends FooAbstract
{
    private $origin;

    public function __construct($origin)
    {
        $this->origin = $origin;
    }

    public function doFoo()
    {
        echo "Doing Foo\n";
        $this->origin->doFoo();
        echo "Done with Foo\n\n";
    }

    public function doBar()
    {
        echo "Doing Bar\n";
        $this->origin->doBar();
        echo "Done with Bar\n\n";
    }
}

Usage change to:

$myFooABC = new LoggingFoo(new FooImplementorABC());
$myFooXYZ = new LoggingFoo(new FooImplementorXYZ());
$myFooABC->doFoo();
$myFooXYZ->doFoo();
$myFooABC->doBar();
$myFooXYZ->doBar();

If you don't want your "client code" to be responsible for the FooImplementor creation, delegate it to a factory (does the concept of enum event exists in PHP ?):

public class FooFactory
{
    public static FooAbstract create(FooType $type)
    {
        switch($type)
        {
            case FooType.ABC:
                return new FooImplementorABC();
            case FooType.XYZ:
                return new FooImplementorXYZ();
            default:
                throw new Exception('Unsupported type ' + $type);
        }
    }
    public static FooAbstract createWithLogging(FooType $type)
    {
        return new LoggingFoo(create($type));
    }
}

Usage becomes:

$myFooABC = FooFactory.createWithLogging(FooType.ABC);
$myFooXYZ = FooFactory.createWithLogging(FooType.XYZ);
  • You make a good point about the SRP. That said, I'm not worried about using a string to determine which class to build. That's the norm in PHP, which does not have a native enum type. Also, I can't hard-code an enum-like set of options, because I need to be able to expand the group of possible implementor classes at runtime. – Ed Cottrell Oct 16 '15 at 13:42
  • @EdCottrell Okay makes sense. Maybe you could complete your question by specifying that the set of implementors can change at runtime. By the way, I find the instruction new $implementorClassName(); really weird ! – Spotted Oct 16 '15 at 14:02

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.