Instanceof code smell

by Thibaud   Last Updated July 18, 2019 11:05 AM - source

The use of instanceof might be a code smell and I am in front of the following code which seems ok. Would you consider that instanceof should not be used in such case? What would be the pattern to use?

<?php

interface Account {}

class PrivateAccount implements Account {}

class PublicAccount implements Account {}

class User {
    private $publicAccounts;
    private $privateAccounts;

    public function hasAccount(Account $account) {
        $haystack = [];

        if($account instanceof PrivateAccount) {
            $haystack = $this->privateAccounts;
        } else if($account instanceof PublicAccount) {
            $haystack = $this->publicAccounts;
        }

        foreach($haystack as $someAccount) {
            if($account->getId() == $someAccount->getId()) {
                return true;
            }
        }
        return false;
    }
}

To be more specific, $privateAccounts and $publicAccounts are objects that will be lazy loaded by an ORM from a relationnal database, so calling getId() is costly (each call results in a database request). PublicAccount and PrivateAccount are 2 tables from the database.

Using $haystack = array_merge($privateAccounts, $publicAccounts) would remove the use of instanceof but would have a performance cost.



Answers 1


Yeah its a code smell to check the type of an object. The whole point of the inheritance is that you shouldn't have to know the type.

In your case the code will break if I pass in some other derived class of Account.

In terms of performance, searching the lazy loaded lists either way is going to be slow. Splitting it down into private and public accounts doesn't really help this in the general case as you may well find that all the users all have only private accounts for example.

Have the database do your FindAccountByUserIdAndAccountId logic, or cache lists of accountIds in the user object to remove this performance issue.

Ewan
Ewan
July 18, 2019 10:58 AM

Related Questions


What differentiates functors from poltergeists?

Updated May 03, 2018 15:05 PM

What should a constructor contain?

Updated February 25, 2018 11:05 AM

Are any side effects not concrete side effects?

Updated April 16, 2015 20:02 PM


Can I say Interface is a set of general behavior?

Updated March 01, 2017 16:05 PM