DEV Community

NotTheBestWay
NotTheBestWay

Posted on

Talk about a little Code Snippet with instanceof

Hello together,

i don't know if this is the right place where i should ask about your Opinion. I want to talk about a little PHP-Code Snippet where i discussed with a Colleague the last days.

The following Code triggered me at an Pull Request Review, and i need your opinion.

Introduction: The following Method should check if the given $foundId is found in the Database AND there are NO mutliple Types which you get from the Database.

public function isValid(int $searchId): bool
{
    $foundIdinDatabase = ArticleRepository::getInstance()->findById($searchId)

    if ($foundIdinDatabase instanceof Article) {
            retrun true;
    }

    return false;
}
Enter fullscreen mode Exit fullscreen mode

The following line triggered me:

if ($foundIdinDatabase instanceof Article) {
Enter fullscreen mode Exit fullscreen mode

I talked to my College and asked, why do you use "instanceof" instead of something like:

if ($foundIdinDatabase) {
    return true;
}
Enter fullscreen mode Exit fullscreen mode

He told me "There is nothing wrong with the code with instanceof, i want to know if i found an Article-Object in the Database? It works correctly and i think there is nothing wrong with this code".
I mentioned that, if as i read the code, i mean that i can get mutliple "Instances of" something like "Book", "Article" or something else because he use "instance of". And thats not the truth, the "search" could return only an (Article) Object or null.

I said, look at the PHP Documentation who to use "instanceof", but nothing happend, the Colleague mean its okay to do it this way.

Whats your Opinion to the code above? Good way, bad way? I'm wrong?

Top comments (0)