Is this the return type covariance issue PHP faced when type declarations launched, violation of Liskov principle, or flaw in my pattern?

softwareengineering.stackexchange https://softwareengineering.stackexchange.com/questions/410926

Question

I've been using a somewhat odd yet effective pattern for a current use case. The one issue is that I'm getting an undefined method notice on a method that is unique to the subclass. The method of course works fine but the notice leads me to believe that either my architecture is shotty or something else is going on.

I don't believe this violates Liskov Principle as the subclass satisfies all contracts with identical signatures, put simply, it could replace any other subclass and work reliably.

Is this an instance of the variance issue PHP has stated when releasing type declarations, a violation of Liskov principle, or do I need to clean up my architecture?

Quick code example and brief explanation.

interface Quiz
{
    public function generate() : void;
}

class LessonQuiz implements Quiz
{
    public function generate() : void
    {
        //stuff here
    }

    public function lessonId() : int 
    {
        //stuff here
    }
}

class QuizClient //Determines/returns instantiated quiz subclass & other permission stuff.
{
    public static function create(string $case) : Quiz
    {
        switch($case){
            case('lesson'): return new LessonQuiz;
            //more logic     
        }
    }
}

/** In another class that builds a lesson template.
* ... 
*/
$quiz = QuizClient::create('lesson');
$lessonId = $quiz->lessonId(); //undefined method.

I have a client that determines and returns a subclass object of an interface. The return type is set to the interface as I need to have some instance of that interface returned.

All subclasses satisfy the interface contracts with identical signatures. However, the subclass specific method is undefined. I don't understand why this is incorrect. Class 'LessonQuiz' is an instance of 'QuizFactory' but also 'LessonQuiz'.

This does work but the fact that it reports method undefined makes me suspicious about my architecture.

Would really appreciate any help.

Was it helpful?

Solution

All subclasses satisfy the interface contracts with identical signatures. However, the subclass specific method is undefined. I don't understand why this is incorrect.

Satisfying the interface contract is only one half of the story. The other half is that clients don't rely on anything other than the said contract. If you write a function that takes a Quiz, you want a guarantee that it works with any implementation that exists today or is created in the future, not just the ones that have a lessonId.

Think about why you need lessonId, what is it that you want to do with it? Do you need to print it? Do you need to use it in some computation? Try and model that generic behavior in the Quiz interface so that it can be implemented in a specific way by each concrete class. If you really cannot find any such behavior without specifically talking about lessons, then perhaps the Quiz interface is not the right abstraction for your use case and you should be dealing directly with the LessonQuiz class.

This does work but the fact that it reports method undefined makes me suspicious about my architecture.

It works because PHP is a dynamically typed language, but designing your application this way undermines the benefit of using interfaces in the first place.

OTHER TIPS

I would say it's a flaw, yes. The issue is that QuizClient::create returns "some type of quiz", not necessarily a LessonQuiz. So if it created a type of quiz that didn't have a lessonId, this code would fail. In fact, in many languages this wouldn't compile at all.

Since it looks like you know when you specifically want a lesson quiz, one possibility would be to add a createLessonQuiz, which either creates a LessonQuiz derivative matching the passed type, or fails if that's not possible, and returns it as a LessonQuiz rather than a Quiz.

Another, if you're using this in a context where it's not known until runtime, is to have a more generate "get metadata" function on Quiz, so you would do something like:

$lessonId = $quiz.getMetadata("lessonId");  // Can be empty!

This is only applicable if "lessonId" is optional information though. If you need lessonId, then you need a LessonQuiz, not a Quiz.

Alright, well I'm an idiot.

I was basing this off of logs from PhpStan while performing static analysis.

Simply checking if the return value was instance of sub class made it pass.

Note to self, always check the simple solutions first.

Licensed under: CC-BY-SA with attribution
scroll top