Thursday, March 03, 2005

Bad usage of instanceof

If ( Object instanceof ClassA || Object instanceof ClassB){
// do something
}

I have come across such code in many applications. Why would developers write such horrendous code? It clearly beats the whole purpose of polymorphism, doesn’t it?

Consider ClassA, ClassB and ClassC implement an interface (say InterfaceA). Wouldn’t the code look more elegant if we add a method to InterfaceA (say doSomething()), implement the method in ClassA and ClassB and provide a dummy implementation in ClassC. Then, the code would look like

Object.doSomething();

The problem with this approach occurs when there are too many implementations of InterfaceA and only a few of them need to implement the method doSomething(). In such a case we could create an Abstract class (say AbstractClassA) which provides a default implementation of the method doSomething(), and override the method only in those few classes. I really think this would be a more elegant solution.

Having said all that, I fail to understand why the instanceof operator was provided in the first place!! Is it only to promote short term hacks like this??

5 comments:

binkley said...

instanceof is still needed. Two obvious situations are serialization so that you can freeze/thaw the exact correct object and parameter narrowing where you need a more exact match than provided an interface or parents method signature.

The best example of the latter is boolean equals(Object) where you want to respond to equals for any possible type, but need to narrow the parameter to your same type for checking equality.

Michael Studman said...

Binkley, I agree with your first point but as to your second I think checking for class equivalence (getClass() == other.getClass()) is preferable to using instanceof in equals methods. Using instance in equals for non-final classes may make your equals method non-symmetric.

E.g.
(new Base().equals(new Derived())
!=
(new Derived().equals(new Base())

Chirdeep Shetty said...

I agree with Micheal in using class equivalence checks in the equals method.
I guess I didn't realize the use of instanceof in serialization.

Naresh Jain said...

I would rather use the following code in my equals()

public boolean equals(Object other) {
if (this == other) return true;
if (null == other) return false;
if (!this.getClass().getName().equals(other.getClass().getName()))
return false;

return equals((MyClassName)other);
}

This is much safer than using instanceOf. Esp. when you have multiple JVMs.

Honza Minárik said...

What if other is a descendant of MyClassName?