Why should use of instanceof be avoided

H

HalcyonWild

Hi,

I read in a magazine (Dev IQ) that finding out the type of object at
runtime is not a good practice. If you are using instanceof a lot, then
your object model should be reviewed. The design of your application
should be changed. I assume the case goes with reflection also.

Why is this so. If anybody has any idea, please share your views.

Thanks.
 
A

Antti S. Brax

I read in a magazine (Dev IQ) that finding out the type of object at
runtime is not a good practice. If you are using instanceof a lot, then
your object model should be reviewed. The design of your application
should be changed. I assume the case goes with reflection also.

This is not a new subject. Search Google.

One of the obvious reasons is that every time you use
instanceof you are hardcoding a class name.
 
H

HalcyonWild

One of the obvious reasons is that every time you use
instanceof you are hardcoding a class name.

I searched google before posting this. Opened the links on the first
few pages. What I found is this:
1. There are some pages which say use of instanceof is ok(javaworld
actually says its good to use instanceof before attempting casting),
and some which say it should be avoided.
2. None of them say hardcoding a class name is the reason.
3. One of them even goes on to say avoid casting, in its list of best
practices.
4. A page from MIT said it indicates clumsy structuring. Not very
enlightening.

One page which I found said that more use of instanceof indicates
poorly designed classes with insufficient use of polymorphism.

What about a scenario where in you want to take different actions on
different subclasses, say SubClass1 and SubClass2, and you are passed
the parent of these two subclasses as parameter in your method. You
have to check which subclass it is exactly. I am using
polymorphism/inheritance here, since I get the parent class.

Where does the wrong design thing come in. The article I originally
read had just a single line on this topic.
 
B

Bjorn Abelli

...
I searched google before posting this. Opened the links on the first
few pages. What I found is this:
1. There are some pages which say use of instanceof is ok(javaworld
actually says its good to use instanceof before attempting casting),
and some which say it should be avoided.

But that's because you otherwise can get a ClassCastException...

It's even better to avoid explicit casting at all, if you can...
2. None of them say hardcoding a class name is the reason.

Well, hardcoding a class name just to make specific actions depending on
class is best to avoid.

Instead it's better to delegate the actions to the classes themselves
(polymorphism).
3. One of them even goes on to say avoid casting,
in its list of best practices.

As I said above... ;-)
4. A page from MIT said it indicates clumsy structuring.
Not very enlightening.

Which means just the same as above, recommending more ploymorphism...
One page which I found said that more use of instanceof
indicates poorly designed classes with insufficient use
of polymorphism.
p4.

What about a scenario where in you want to take different
actions on different subclasses, say SubClass1 and SubClass2,
and you are passed the parent of these two subclasses as
parameter in your method. You have to check which subclass
it is exactly. I am using polymorphism/inheritance here, since
I get the parent class.

No, you're not really using the "Polymorphism pattern" (look up GRASP), as
you even need to know which subclass it is.

The object know which class it is, so behaviour depending on which class it
is, should be delegated to the object itself.

// Bjorn A
 
P

Paul Tomblin

In a previous article said:
What about a scenario where in you want to take different actions on
different subclasses, say SubClass1 and SubClass2, and you are passed
the parent of these two subclasses as parameter in your method. You
have to check which subclass it is exactly. I am using
polymorphism/inheritance here, since I get the parent class.

If you want action "foo" to happen in SubClass1 and action "bar" to happen
in SubClass2, then the parent class should provide a method which SubClass
overrides to do "foo" and SubClass2 overrides to do "bar'. Then you don't
have "instanceof" calls, although the converse is that if you're doing
this too much you end up with bazillions of little action methods that do
very little.
 
T

Thomas Weidenfeller

HalcyonWild said:
I searched google before posting this. Opened the links on the first
few pages. What I found is this:

instanceof is some kind of programmer's duct tape. You should try to
avoid it, and use real nuts and bolts in your code, but if you can't
avoid it you are glad that you have it.

In practice, this means if the design of some new code relies on
instanceof you should better think twice and look for other solutions.
If you have to "somehow" interface to existing code out of your control,
instanceof can be the thing that saves you.

1. There are some pages which say use of instanceof is ok(javaworld
actually says its good to use instanceof before attempting casting),

IMHO you interpreted this in the wrong way. I bet, the javaworld author
said or wanted to say:

If(!) you are about to cast an object of totally unknown type,
it is a good idea to check with instanceof first. This way you
can avoid having to deal with the class cast exception.

This is indeed one of these duct tape things. One is usually unsure of
the real type of an object if it is created by some part (library, etc.)
of the application outside of the own control. So you somehow figure out
the real type, and then you cast to it - but, as a safeguard you better
check.

If you are unsure of the type of an object in your own code, you better
revise your own code, instead of adding safeguards.
and some which say it should be avoided.

I am in that camp.
2. None of them say hardcoding a class name is the reason.

It is one of the things which makes instanceof impractical.
3. One of them even goes on to say avoid casting, in its list of best
practices.

Which is indeed good advice. Unfortunately the standard API and classes
often force you to do some casting. This got better with Java 1.5 where
e.g. the casting needed for the collection classes can be replaced by
using generics.

Still, in your own code avoiding casting is a good idea.
4. A page from MIT said it indicates clumsy structuring. Not very
enlightening.

They are right. It is exactly that. If you feel the need to use an
instanceof in your code, some little alarm bell in your head should ring
and you should investigate your code for a better solution.
One page which I found said that more use of instanceof indicates
poorly designed classes with insufficient use of polymorphism.

Yes, again they are right. Making use of polymorphism is often a better
design alternative to using an instanceof. Using instanceof in fact
often indicates that the programmer didn't get OO programming very well.
What about a scenario where in you want to take different actions on
different subclasses, say SubClass1 and SubClass2, and you are passed
the parent of these two subclasses as parameter in your method.

Bzzzz, game over, you lost.

(1) Instead of

void myMethod(Superclass o) {
if(o instanceof SubclassA) {
// do something specific for SubclassA
} else if(o instanceof SubclassB) {
// do something specific for SubclassB
}
}

you write (2)

void myMethod(SubclassA o) {
// do something specific for SubclassA
}

void myMethod(SubclassB o) {
// do something specific for SubclassB
}

(3) Or, sometimes it makes more sense to implement the particular method
as part of the object itself, overriding a Superclass method:

void myMethod(Superclass o) {
o.doSomething();
}

You
have to check which subclass it is exactly.

No, you leave this to the compiler.
I am using
polymorphism/inheritance here, since I get the parent class.

You use dynamic polymorphism with static type checking. Already the
usage of the two words "dynamic" and "static" in the same sentence
should ring a bell. Using a separate method for each subtype (2) fixes
the problem with ad-hoc polymorphism (overloading). Using overridden
methods and double-dispatch (3) is another possible fix.
Where does the wrong design thing come in.

From your idea that you, yourself absolutely have to check the type of
an object if you need to do some type-specific operation on that object.

/Thomas
 
S

shakah

HalcyonWild wrote:
[...stuff deleted...]
What about a scenario where in you want to take different actions on
different subclasses, say SubClass1 and SubClass2, and you are passed
the parent of these two subclasses as parameter in your method. You
have to check which subclass it is exactly. I am using
polymorphism/inheritance here, since I get the parent class.

Where does the wrong design thing come in. The article I originally
read had just a single line on this topic.

The basic idea is that rather than the following:
public abstract class MyShape {
}

public class MySquare extends MyShape {
public float fEdgeLength_ ;
}

public class MyCircle extends MyShape {
public float fRadius_ ;
}

public class YourClass {
public float calcArea(MyShape s) {
if(s instanceof MySquare) {
return ((MySquare) s).fEdgeLength_ * ((MySquare)
s).fEdgeLength_ ;
}
else if(s instanceof MyCircle) {
return 3.14159f * ((MyCircle) s).fRadius_ * ((MyCircle)
s).fRadius_ ;
}
return 0.0f ;
}
}

it is better to do something like:
public abstract class MyShape {
public float calcArea() ;
}

public class MySquare extends MyShape {
private float fEdgeLength_ ;
public float calcArea() {
return fEdgeLength_ * fEdgeLength_ ;
}
}

public class MyCircle extends MyShape {
private float fRadius_ ;
public float calcArea() {
return 3.14159f * fRadius_ * fRadius_ ;
}
}

public class YourClass {
public float calcArea(MyShape s) {
return s.calcArea() ;
}
}
 
J

John C. Bollinger

HalcyonWild said:
I searched google before posting this. Opened the links on the first
few pages. What I found is this:
1. There are some pages which say use of instanceof is ok(javaworld
actually says its good to use instanceof before attempting casting),
and some which say it should be avoided.

There are cases in which casting is unavoidable and in which you cannot
be confident that the object to be cast is in fact of the type you want.
Implementation of an equals(Object) method is a good example. In
these cases, instanceof is pretty much indispensable. Note well,
however, that even here, the need for instanceof is construed by many as
a language wart.
2. None of them say hardcoding a class name is the reason.

Well what reasons do they give, then? It seems that would be a fair
place to start the discussion. The need to hardcode a class name _is_ a
good reason to avoid instanceof, particularly when the class name in
question is not that of the class in which the name is to be used.

More generally though, relying on instanceof is generally a sign of poor
OO practices. The _declared_ type of an Object should be sufficient for
your program's use within the scope of that declaration. If it isn't,
then you need either a more specific declaration or a better design for
the class / interface corresponding to the declared type.

This is not just theoretical. A program that relies on run-time type
information is often brittle relative to a similar program that does
not. The former does not usually adapt well to new classes being added,
and it can be a real mess to determine what a method can, should, or
will do without deep analysis of program flow.
3. One of them even goes on to say avoid casting, in its list of best
practices.

Absolutely. That goes along with my comments above. It was much harder
to do that before Java 1.5, but now, with proper use of Generics, a
well-written program can avoid most casts.
4. A page from MIT said it indicates clumsy structuring. Not very
enlightening.

I suspect that it was referring again back to the same OO issues that I
raised. You should not need to know anything more specific about an
object's type than the applicable type declaration tells you. That
could be taken as a minimalist definition of polymorphism.
One page which I found said that more use of instanceof indicates
poorly designed classes with insufficient use of polymorphism.

Yes. Once again the same thing.
What about a scenario where in you want to take different actions on
different subclasses, say SubClass1 and SubClass2, and you are passed
the parent of these two subclasses as parameter in your method. You
have to check which subclass it is exactly. I am using
polymorphism/inheritance here, since I get the parent class.

If you cannot reasonably give the appropriate SubClass1 and SubClass2
methods the same name, arguments, and return type, then either your
class hierarchy is messed up or your method is ill-conceived. You are
NOT correctly using polymorphism here; if you were, then you would not
say that you want to "take different actions on different subclasses".
Read that again, because it is KEY to using polymorphism effectively.
Where does the wrong design thing come in. The article I originally
read had just a single line on this topic.

Once again, the same thing.
 
R

Ross Bamford

What about a scenario where in you want to take different actions on
different subclasses, say SubClass1 and SubClass2, and you are passed
the parent of these two subclasses as parameter in your method. You
have to check which subclass it is exactly. I am using
polymorphism/inheritance here, since I get the parent class.

I would make the 'parent' an interface. Not necessarily *the* interface,
mind you (i.e. it could be a specialised interface that your object
implements among other things). What I mean is you don't have to have
'MyObjectInter' and 'MyObjectImpl'.

If you have subclasses that provide different methods depending on type,
and you need to call those from /above/ in the hierarchy, your design is
broken. Your design isn't Object Oriented, it's just written as Objects.
So you might have a 'DataEater' class, which has a method that swallows
data, and two subclasses, 'CachingEater' and 'BitBucketEater'. Each of
these will have it's own methods (flushcache(), kickbucket(), etc), and
that is fine. Wherever you need to access them, you pass in the actual
type.

The whole point of Encapsulation, though, is that if I'm given a
DataEater I can blindly give it by data and expect it to be eaten.
Contrast this with:

public cacheOrKickEater(DataEater eater) {
if (eater instanceof CachingEater)
((CachingEater)eater).flushCache();
else
((BitBucketEater)eater).kickBucket();
}

In the context of the points above, you see how unnatural this is. It's
is written with Objects, but it's not really OOP.

(n.b. this is not working code, it would likely generate cast
exceptions, especially when you extended your hierarchy).

Ross
 
K

Knute Johnson

HalcyonWild said:
I searched google before posting this. Opened the links on the first
few pages. What I found is this:
1. There are some pages which say use of instanceof is ok(javaworld
actually says its good to use instanceof before attempting casting),
and some which say it should be avoided.
2. None of them say hardcoding a class name is the reason.
3. One of them even goes on to say avoid casting, in its list of best
practices.
4. A page from MIT said it indicates clumsy structuring. Not very
enlightening.

One page which I found said that more use of instanceof indicates
poorly designed classes with insufficient use of polymorphism.

What about a scenario where in you want to take different actions on
different subclasses, say SubClass1 and SubClass2, and you are passed
the parent of these two subclasses as parameter in your method. You
have to check which subclass it is exactly. I am using
polymorphism/inheritance here, since I get the parent class.

Where does the wrong design thing come in. The article I originally
read had just a single line on this topic.

It's in the language and it isn't going to go away so use it if you need
it. If it makes your program easier to read or quicker to code who
really cares?
 
E

Eric Sosman

John said:
HalcyonWild wrote
[quoting someone else]
One of the obvious reasons is that every time you use
instanceof you are hardcoding a class name.

I searched google before posting this. Opened the links on the first
few pages. What I found is this:
[...]
2. None of them say hardcoding a class name is the reason.

Well what reasons do they give, then? It seems that would be a fair
place to start the discussion. The need to hardcode a class name _is_ a
good reason to avoid instanceof, particularly when the class name in
question is not that of the class in which the name is to be used.

Is the need to hard-code a class name also a good reason
to avoid `new'?

There are good reasons to minimize the use of `instanceof',
but IMHO "avoiding hard-coded class names" is not among them.
 
J

John C. Bollinger

Eric said:
Is the need to hard-code a class name also a good reason
to avoid `new'?

It is the *only* reason I see to actively avoid 'new'. You can employ
factory classes, and, to a lesser extent, factory methods to reduce the
number appearances of class names in your code and the impact of naming
classes specifically when you create objects. In many cases this is
very much overkill, however.

It is considerably more reasonable to tie creation of an object to a
specific, hard-coded class name than it is to tie program logic to the same.
There are good reasons to minimize the use of `instanceof',
but IMHO "avoiding hard-coded class names" is not among them.

As you like. I have no particular investment in my opinion on the
matter, and so no special interest in trying to persuade you to change
yours.
 
F

Fred L. Kleinschmidt

HalcyonWild said:
Hi,

I read in a magazine (Dev IQ) that finding out the type of object at
runtime is not a good practice. If you are using instanceof a lot, then
your object model should be reviewed. The design of your application
should be changed. I assume the case goes with reflection also.

Why is this so. If anybody has any idea, please share your views.

Thanks.

Many responders have pointed out some of the reasons to minimize the us
of instanceof, but there are many times when it is your friend.

Consider a GUI application that needs to determine the Window (or
Dialog, or Frame, etc.) ancestor of a GUI component. The easiest way is
to walk the parent tree up until finding a parent that is "instanceof
Window".
 
A

Anton Spaans

HalcyonWild said:
Hi,

I read in a magazine (Dev IQ) that finding out the type of object at
runtime is not a good practice. If you are using instanceof a lot, then
your object model should be reviewed. The design of your application
should be changed. I assume the case goes with reflection also.

Why is this so. If anybody has any idea, please share your views.

Thanks.

From the arguments in the previous posts, it is best to avoid casts and
instanceof statements as much as possible.
But sometimes it can have its use.

One situation is the use of so-called 'Marker Interfaces'. An example is
java.io.Serializable.
Such an interface does not declare any methods. Instead it declares a
contract between objects implementing the marker-interface and the users of
such objects and this type of contract can not be expressed with
declarations, using java syntax.

As an example: how would one express the contract of java.io.Serializable
in plain java-code? (the contract stating - in short - that objects
implementing java.io.Serializable must have members that are either
serializable as well or that are volatile)

The user of objects implementing marker-interfaces make use of the
instanceof statement.

What do you all think about using these marker-interfaces?

URL to definition: http://tinyurl.com/djr49
-- Anton.
 
L

Lee Weiner

What about a scenario where in you want to take different actions on
different subclasses, say SubClass1 and SubClass2, and you are passed
the parent of these two subclasses as parameter in your method. You
have to check which subclass it is exactly. I am using
polymorphism/inheritance here, since I get the parent class.

Inheritance, yes. Polymorphism, no. If you were using polymorphism
correctly, you would have methodA() in SubClass1 and methodA() in SubClass2
and each would know the correct thing to do to its object.

Lee Weiner
lee AT leeweiner DOT org
 
W

Wibble

I think avoiding hardcoding of class names is a silly reason not to use
instance of. The reason not to use it is because objects should
describe thier own behavior. If you move all the behavior out of the
object into scattered instanceof's, then its very hard to figure out
what the object actually does. The reason instanceof is fine in an
equals method, is that its looking for an instance of the current
class, still describing its own behavior.
 
D

Dale King

Thomas said:
Bzzzz, game over, you lost.

(1) Instead of

void myMethod(Superclass o) {
if(o instanceof SubclassA) {
// do something specific for SubclassA
} else if(o instanceof SubclassB) {
// do something specific for SubclassB
}
}

you write (2)

void myMethod(SubclassA o) {
// do something specific for SubclassA
}

void myMethod(SubclassB o) {
// do something specific for SubclassB
}

That is not really equivalent. Consider if I have this calling code:

// someMethod returns SubclassA or SubclassB instances
Superclass x = someMethod();
myMethod( x );

Will the correct method be called? No, in fact the code won't compile.
While the choice of which class' *overridden* method to call is based on
the runtime type of the object, the choice of which *overloaded* method
to call is based solely on the compile-time types of the arguments.

Your solution will only work if every caller knows the type of the object.
 
A

Antti S. Brax

It's in the language and it isn't going to go away so use it if you need
it. If it makes your program easier to read or quicker to code who
really cares?

And who cares if it makes your program a nightmare to maintain?

You will as soon as you meet code that has been written by
someone else who thinks like you. :)
 
M

Michael Rauscher

Knute Johnson wrote on the usage of "instanceof":
It's in the language and it isn't going to go away so use it if you need
it.

The point is: in which cases does one *need* it?

It's in the language, right. Would you also advise, that a C programmer
should use "goto" just because it's in the language?

In fact, the usage of "instanceof" is not (only) a programming issue or
a matter of coding style. It indicates that one circumvents polymorphism
which in turn indicates the possibility of having problems within the
design. In some way this applies to casts, too.

Therefore the problem isn't the "instanceof" itself. E.g. take a look at
Object#equals. If you override this, you'll have to use "instanceof" and
to cast in most cases. On the other hand, a universal action listener

public void actionPerformed(ActionEvent e ) {
Object source = e.getSource();
if ( source instanceof JMenuItem )
...
else if ( source instanceof JButton )
...
else if ( source instanceof JTextField )
...
}

is a serious design issue.

Bye
Michael
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,755
Messages
2,569,537
Members
45,023
Latest member
websitedesig25

Latest Threads

Top