(Please no advise that I should abstract X
more and add another method to it.)
In C++, when I have a variable x
of type X*
and I want to do something specific if it is also of type Y*
(Y
being a subclass of X
), I am writing this:
if(Y* y = dynamic_cast<Y*>(x)) {
// now do sth with y
}
The same thing seems not possible in Java (or is it?).
I have read this Java code instead:
if(x instanceof Y) {
Y y = (Y) x;
// ...
}
Sometimes, when you don't have a variable x
but it is a more complex expression instead, just because of this issue, you need a dummy variable in Java:
X x = something();
if(x instanceof Y) {
Y y = (Y) x;
// ...
}
// x not needed here anymore
(Common thing is that something()
is iterator.next()
. And there you see that you also cannot really call that just twice. You really need the dummy variable.)
You don't really need x
at all here -- you just have it because you cannot do the instanceof
check at once with the cast. Compare that again to the quite common C++ code:
if(Y* y = dynamic_cast<Y*>( something() )) {
// ...
}
Because of this, I have introduced a castOrNull
function which makes it possible to avoid the dummy variable x
. I can write this now:
Y y = castOrNull( something(), Y.class );
if(y != null) {
// ...
}
Implementation of castOrNull
:
public static <T> T castOrNull(Object obj, Class<T> clazz) {
try {
return clazz.cast(obj);
} catch (ClassCastException exc) {
return null;
}
}
Now, I was told that using this castOrNull
function in that way is an evil thing do to. Why is that? (Or to put the question more general: Would you agree and also think this is evil? If yes, why so? Or do you think this is a valid (maybe rare) use case?)
As said, I don't want a discussion whether the usage of such downcast is a good idea at all. But let me clarify shortly why I sometimes use it:
Sometimes I get into the case where I have to choose between adding another new method for a very specific thing (which will only apply for one single subclass in one specific case) or using such instanceof
check. Basically, I have the choice between adding a function doSomethingVeryVerySpecificIfIAmY()
or doing the instanceof
check. And in such cases, I feel that the latter is more clean.
Sometimes I have a collection of some interface / base class and for all entries of type Y
, I want to do something and then remove them from the collection. (E.g. I had the case where I had a tree structure and I wanted to delete all childs which are empty leafs.)
Now, I was told that using this castOrNull function in that way is an evil thing do to. Why is that?
I can think of a couple of reasons:
It is an obscure and tricky way of doing something very simple. Obscure and tricky code is hard to read, hard to maintain, a potential source of errors (when someone doesn't understand it) and therefore evil.
The obscure and tricky way that the castOrNull
method works most likely cannot be optimized by the JIT compiler. You'll end up with at least 3 extra method calls, plus lots of extra code to do the type check and cast reflectively. Unnecessary use of reflection is evil.
(By contrast, the simple way (with instanceof
followed by a class cast) uses specific bytecodes for instanceof and class casting. The bytecode sequences can almost certainly will be optimized so that there is no more than one null check and no more that one test of the object's type in the native code. This is a common pattern that should be easy for the JIT compiler to detect and optimize.)
Of course, "evil" is just another way of saying that you REALLY shouldn't do this.
Neither of your two added examples, make use of a castOrNull
method either necessary or desirable. IMO, the "simple way" is better from both the readability and performance perspectives.