Is it correct way of use LSP in PHP?

Question

<?php

class AnimalFood
{
    public function prepare(): string
    {
        return 'Prepare animal food.'."\n";
    }
}

class PetFood extends AnimalFood
{
    public function prepare(): string
    {
        return 'Prepare pet food.'."\n";
    }

    public function getProducer(): string
    {
        return 'PetFruitLTD.';
    }
}

class Animal
{
    public function feed(AnimalFood $food): Animal
    {
        echo $food->prepare();
        echo 'Feeding...'."\n";
        return $this;
    }
}

class Pet extends Animal
{
    public function feed(AnimalFood $inputFood): Pet
    {
        $food = $this->getFood($inputFood);
        parent::feed($food);
        echo sprintf('Producer of pet food was %s'."\n", $food->getProducer());
        return $this;
    }

    private function getFood(AnimalFood $food): PetFood
    {
        return $food;
    }
}

$pet = new Pet();
$petFood = new PetFood();
$pet->feed($petFood);

/**
Prepare pet food.
Feeding...
Producer of pet food was PetFruitLTD.
*/

Of course the title question corresponds to the getFood method in class Pet. I don't know is it correct to achieve that result that when I get in argument of base class (method getFood in Pet), then I invoke method to get correct type. Type where I will know that it has method I'am looking for. This is not a abuse of LSP? I ask because it is the only one way I know which can make static analysis makes possible to pass eg. in VSCode. Perhaps there other workarounds? Please help.

Your `getFood` method is technically not incorrect, but it also doesn’t do anything, except, as you noted, appease a static analysis tool which could instead be done with a comment. — Chris Haas, Jul 11 '22 at 12:34
I think that you found the solution. I need use the comments. Is it your answer @Chris Haas ? Simply use: `/** @var PetFood $food */ $food = $this->getFood($inputFood)` I think i would be great helpful if you answer and give an exact example with place where it should be placed (this comment with type) with the context of given in question snippet. — Marek G., Jul 11 '22 at 13:06

score 1 · Accepted Answer · answered Jul 11 '22 at 13:46

Converting my comment to a answer

Introducing a function/method that takes a type and returns a narrower version is technically following LSP, however if no additional work is done, this kind of code is really only doing it to make a static analysis happy, and there is no cast happening. Instead, you are making code that exists only for tooling and are increasing your code debt. Small things like this aren't the end of the world, and your method is also private, but it is a pattern that I'd recommend avoiding unless you need it for your code to actually run.

    // Doesn't really do anything
    private function getFood(AnimalFood $food): PetFood
    {
        return $food;
    }

This code is obviously contrived for example purposes, so it is hard to argue about things exactly as written. In the real world, if getFood() is actually needed to perform additional work, that code is valid however I'd include type checks to guard:

    private function getFood(AnimalFood $food): PetFood
    {
        if (!$food instanceof PetFood) {
            throw new RuntimeException('Pet was fed non-pet food');
        }
        
        // Do work
        // ...

        return $food;
    }

If however you don't need to perform work, then I'd drop the getFood() method completely and use a DocBlock var

class Pet extends Animal
{
    public function feed(AnimalFood $food): Pet
    {
        /** @var PetFood $food */
        parent::feed($food);
        echo sprintf('Producer of pet food was %s'."\n", $food->getProducer());

        return $this;
    }
}

But, to go one final step, any good static analysis tool should be able to handle type checks at the code level, too, so you can perform a first-class runtime type check and makes both your tooling happy, as well as guards your code from invalid paths.

class Pet extends Animal
{
    public function feed(AnimalFood $food): Pet
    {
        if (!$food instanceof PetFood) {
            throw new RuntimeException('Pet was fed non-pet food');
        }

        parent::feed($food);
        echo sprintf('Producer of pet food was %s'."\n", $food->getProducer());

        return $this;
    }
}

One minor change I also made was to keep the method parameter named the same since PHP 8 introduced named arguments. This isn't required and not LSP, but I think it is a good idea to stay consistent if you want your class/objects to be thought of as interchangeable.

Is it correct way of use LSP in PHP?

1 Answers1