0

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.

3 Answers 3

4

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.

1
  • 1
    Marking this as the correct answer as it read through the lines and got to the bigger issue at hand. If we're enforcing strict types, then we need to be consistent with them. That means the correct specificity of a subclass object with a unique method, despite being in an interface. I updated the concretes & clients to either fit firmly with the contracts if return type is set to interface and concrete class type if unique methods are used. Thank you for the perspective, it was helpful.
    – Rich Davis
    Jun 4, 2020 at 16:38
2

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.

2
  • First off, I really appreciate the answer. It's always very beneficial to get insight from those who write in other languages (I apologize if this assumption is incorrect), it puts the structure of architecture in a broader context. In php, we have interfaces which serve as a tool to fill in a gap that's natively enforced by compiled languages. An interface specifies abstract methods that must be implemented by subclasses that implement the interface (the 'generate' method above). These subclasses can have additional unique methods however ('lessonId') above.
    – Rich Davis
    Jun 2, 2020 at 2:40
  • .. Ran out of words here. So the code works fine but static analysis reveals the warning about the method not being found. I suppose the better question to ask is is there something wrong with typehinting a return value as the interface when that value will perform unique subclass methods by the client? I know I want the subclass to be an instance of the interface from the 'create' method. I could have individual methods in the client returning each subclass typehinting the concete class but that I worry my co-workers wouldn't intuitively adhere to the contracts and pattern.
    – Rich Davis
    Jun 2, 2020 at 2:43
-1

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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.