Liskov substitution principle and proper way to use inherited classes

Question

I have some handler ("controller") classes and they can process items in some way:

interface IHandler
{
    public function execute(Item $item);
}

class FirstHandler implements IHandler
{
    public function execute(Item $item) { echo $item->getTitle(); }
}

class SecondHandler implements IHandler
{
    public function execute(Item $item) { echo $item->getId() . $item->getTitle(); }
}

class Item
{
    public function getId() { return rand(); }
    public function getTitle() { return 'title at ' . time(); }
}

But then I need to add some new functionality in child Item class:

class NewItem extends Item
{
    public function getAuthor() { return 'author ' . rand(); }
}

and use it in SecondHandler

class SecondHandler implements IHandler
{
    public function execute(Item $item) { printf('%d %s, author %s', $item->getId(), $item->getTitle(), $item->getAuthor()); }
}

But Item class actually has not getAuthor method. And, if I try to change signature of accept method in SecondHandler class, I will catch E_STRICT error about declaration compatibility. And, of course, it's sort of LSP violation.

How can I fix this problem? Do I need two interfaces, for example, INewHandler and IHandler, with different signatures of execute method? But it's some sort of code duplicates.

Also, I cannot use __constructor(Item $item) and __construct(NewItem $item) in handlers (and execute method without arguments), which will be seen like a better solution: they must be immutable and only single instance of every strategy allowed in application lifecycle.

Answer 1

As you discovered by yourself, the type hinting implementation of PHP has a lot limitations that make scenarios, like the one described by you, harder than they should be. In other typed languages like Java and Swift your implementation is absolutely licit.

After some thinking on your question I came to the solution presented by Félix but I consider it too much over engineered compared to the problem.

My answer to your question is not a solution but an advice that I give to you after years of development with PHP:

Give up with type hinting in PHP and develop like it should be... in a dynamic way.

PHP is more similar to Ruby/Python/JavaScript than Java/C++, and trying to copy 1 to 1 from static typed languages translates in forced and convolute implementations.

The solution to your implementation problem is easy, so don't over complicate it and keep it easy as it should be (KISS principle).

Declare the methods' arguments without the type and implement a check where you really need (for example throwing an exception).

interface IStrategy
{
    public function execute($item);
}

class FirstStrategy implements IStrategy
{
    public function execute($item) {
        echo $item->getTitle();
    }
}

class SecondStrategy implements IStrategy
{
    public function execute($item) {
        // execute(NewItem $item) is identical to this check.
        if (! $item instanceof NewItem) {
            throw new Exception('$item must be an instance of NewItem');
        }
        echo $item->getAuthor();
    }
}

class Item
{
    public function getId() { return rand(); }
    public function getTitle() { return 'title at ' . time(); }
}

class NewItem extends Item
{
    public function getAuthor() { return 'author ' . rand(); }
}

Again, don't think in Java but follow as much as possible the duck typing way.

When possible, try to don't strictly force the type of the parameters but adapt the behavior of the code based on the available interfaces (Duck Typing).

class SecondStrategy implements IStrategy
{
    public function execute($item) {
        $message = $item->getTitle();

        // PHP 5 interface availability check.
        if (is_callable([$item, 'getAuthor'])) {
            $message .= ' ' . $item->getAuthor();
        }

        // With PHP 7 is even better.
        // try {
        //     $message .= ' ' . $item->getAuthor();
        // } catch (Error $e) {}

        echo $message;
    }
}

I hope to have helped you. ^_^

Answer 2

Both @daniele-orlando and @ihor-burlachenko made valid points. Consider following approach for method overloading, which is kind of a compromise and should scale well:

interface IHandler
{
    /**
     * @param $item Item|NewItem
     */
    public function execute($item);

    // protected function executeItem(Item $item);
    // protected function executeNewItem(NewItem $item);    
}

trait IHandlerTrait
{   
    public function execute($item) 
    {
        switch(true) {
            case $item instanceof Item:
                return $this->executeItem($item);
            case $item instanceof NewItem:
                return $this->executeNewItem($item);
            default:
                throw new \InvalidArgumentException("Unsupported parameter type " . get_class($item));
        }
    }

    protected function executeItem(Item $item)
    {
        throw new \LogicException(__CLASS__ . " cannot handle execute() for type Item");
    }

    protected function executeNewItem(NewItem $item)
    {
        throw new \LogicException(__CLASS__ . " cannot handle execute() for type NewItem");
    }
}

class FirstHandler implements IHandler
{
    use IIHandlerTrait;

    protected function executeItem(Item $item) { echo $item->getTitle(); }
}

class SecondHandler implements IHandler
{
    use IIHandlerTrait;

    // only if SecondHandler still need to support `Item` for backward compatibility
    protected function executeItem(Item $item) { echo $item->getId() . $item->  getTitle(); }

    protected function executeNewItem(NewItem $item) { printf('%d %s, author    %s', $item->getId(), $item->getTitle(), $item->getAuthor()); }
}