Tuesday, August 22, 2006

Tricky instanceof operator

Let's start with a little puzzle.

Object obj = ...
System.out.println(obj instanceof Object);

How do you initialize the obj in order to print "false"?

Well, aren't all possible Java objects instances of Object? To answer this question one must understand how instanceof operator works. The answer is that it would print "true" for any Object. The only way to make it false is not to give it an object, give it a null reference.

The tricky bit about instanceof operator can be that if object on the left side is null, the condition evaluates to false. Therefore there is no need for null check after a class cast as in the following example.

public boolean equals(Object o) {
if (o instanceof MyClass) {
MyClass mc = (MyClass) o;
if (mc == null) // never null
return false;
else
return ... // compare members of mc
}
return false;
}

This can be simplified as shown in the following code snippet

public boolean equals(Object o) {
if (o instanceof MyClass) {
MyClass mc = (MyClass) o;
return ... // compare members of mc
}
return false;
}

So the moral if this excercise is that instanceof operator works as you would expect with objects, and returns false when given a null.

And remember that equals() method is probably the only reasonable place for instanceof operator. If you do it elsewhere and use it to create an alternative flow (e.g. if-else, switch) it is a bad smell and should be replaced with polymorphism. Read more about Swich Statement code smell and Polymorphism

BTW the previous example was not a very nice example of how to implement equals method, so do not copy it ;-) Usually we would do something like this

public boolean equals(Object o) {
if (o == null) return false;
if (o == this) return true;
if (!(o instanceof MyClass)) return false;
MyClass mc = (MyClass) o;
return ... // compare members of mc
}

15 comments:

carlo*10 said...

Hey great blogsite. I found your website to be really informative and helpful. In fact, I did learn a few things after looking through some of the articles posted here. Keep up the good work, thanks.

Dushan Hanuska said...

Thanks, mate!

I'm pleased to hear you (and hopefully many others) appreciate my blogs.

Anonymous said...

So does that mean that the first check for null in an equals method is not nessacary?

Dushan Hanuska said...

Yes you are right, the first comparison for null is not necessary as the instanceof operator will take care of it.

CheGuara said...

I have a little problem with your last statement that instanceof should be used in only equals.

Lets say there is a jar file which contains an abstract class A. We want to use this jar file and A class in our code so that we can extend A to two different classes B and C. In A has an abstract method which needs to be implemented in B and C, but B and C expects different kind of objects as input to that function. So i think the only option is to pass object in abstract method and do instanceof checks in implementation in B and C.
Lemme know what do you think

Dushan Hanuska said...

I think that you might have a problem with the design. If you need to implement classes B and C in a way that they depend on the implementation of the object passed in as a parameter.

Inheritance should give you the ability to do different things to same objects.

If you need to find out the implementation first (using instanceof operator), and then behave accordingly - it is a "smell".

I can see the problem in two places here:
1) in classes B and C: maybe you should use different method (one in B, another in C class) for these two types (method overloading might help). If you need to do something different on each time, maybe it's better to pass these into different methods.
2) in the objects received as parameters: if you need to call different methods on these objects, consider having a common method that delegates the call to a desired method.

Which method is better depends on what exactly you are trying to do differently.

Let me know if this helps. If I wasn't clear enough I can try to give you some examples (code examples).

Will said...

>> If you need to find out the implementation first (using instanceof operator), and then behave accordingly - it is a "smell".

I agree that instanceof is bad if you're using it to test for implementation, but what if you need to figure out if an object implements an interface?

Consider a user account system, which has two different kinds of users, Person and Business:

interface User {...};

interface Person extends User {
Date getBirthDate();
...
}

interface Business extends User {
Person getPresident();
...
}

If you have logic that depends on whether a user is a Person or a Business, it seems appropriate to use 'instanceof' here. The alternative using polymorphism requires methods in the super-interface for determining what sub-interfaces are implemented. Adding new kinds of Users would require adding more methods or enum constants to User.

Or is it poor design to treat people and businesses differently? :-D

Dushan Hanuska said...

Will, thank you for your comment! It actually makes a good point.

There are many applications where we have seen similar approach. Inheritance is good, don't get me wrong, but it's not the answer to all problems of code extensibility.

Your code sample is a typical example, where the code would greatly benefit from composition rather than a bit out-stretched Business-User inheritance. It's hard to say that a business is a type of a person. You could easily say that a business can consist of one or more people, but it is not same entity type.

If you had Person and Business objects that do not inherit from same root, you could overload a method to accept one or the other - just another way to avoid instanceof operator.

Anonymous said...

Is this bad design? -

I have a base abstract class BaseClass that specifies some methods including validate().

All access to my logic is through a single Class which checks: if (A instanceof BaseClass) A.validate();
Where the validation would change depending on the actual class extending BaseClass.

Am I naive?

Dushan Hanuska said...

I cannot see the other cases in your short example therfore it is hard to argue for or against what you are saying.

I don't see the reason for you to use instanceof operator. If every class is a sub-class of the BaseClass and implements its own validate() method you do not need to check for its class type. Simple call validate() method and it'll perform validation specific to its own implementation.

However, if some of your classes extend other base class as well you have a case of mixing pears with apples. In such case, I would suggest for your base classes to implement a common interface, which can be as simple as a validate() method. Then you will not need to check for class type or cast your objects to a particular class.

giftiger_wunsch said...

Objects in java are case-sensitive. "system.out.println" will not work.

Dushan Hanuska said...

Thank you Giftiger! I corrected it.

Anonymous said...

Thank You for the information!

Anonymous said...

I ran across this blog when thinking about my use of the instanceof operator and decided to try a different approach. Thing is, I can't think of another way, so could you please point me in the right direction?

I have Person that have their emailaddresses stored in different places, so I created different extends and when looking up the emailaddress I did the instanceof to determine which lookup implementation to use.
I was thinking of having Person implement IEmail for String getEmail() and create a IEmailProvider but how do I hook these together? How do I know which one to use?

Thanks,
David

Anonymous said...

Nice article. One question though: how come you first explain why it is not necessary to check for null when using instanceof, but within your "recommended" equals() you do it anyhow?


Creative Commons License This work is licensed under a Creative Commons Attribution-NonCommercial-ShareAlike 2.5 License.