I think for the most part we're following good design principals. The
use of instanceof in this case is because we don't know the object's
type at compile time, and that happens often in the script interpreter code.
Take the new converter code as an example. It uses Gang of Four design
patterns and one of Martin Fowler's refactoring methods, yet it still
needs to use instanceof. If you have Object A that is an unknown type,
and you need to convert it to Object type B, you have to use instanceof
to determine the type of Object A in order to lookup the correct converter.
-Adrian
Tim Ruppert wrote:
Everybody's got their own style dude - I could find you 10 articles that
will flip it. Besides - there are no absolutes in this world, so I'll
refrain from spending my day trying to prove that the instance that you
were referring to IS a bad practice.
When writing object oriented code I also think it's generally a "bad"
practice to just use Object, but instead tend to not use that catch all
as the way I write my code - but hey that's just me. I think better
thought thru designs yield cleaner interfaces that doing things like
"instanceof" and setting everything to object.
Just my two cents - feel free to drop them on the floor (face up) for
the next lucky person walking by :)
Cheers,
Ruppert
--
Tim Ruppert
HotWax Media
http://www.hotwaxmedia.com
o:801.649.6594
f:801.649.6595
On Nov 24, 2009, at 11:09 AM, Adam Heath wrote:
Tim Ruppert wrote:
Instance of is a bad practice to get into in object oriented code -
might as well just go back to C and switch statements.
==
String foo = (String) context.get("foo");
if (UtilValidate.isEmpty(foo)) {
}
Object bar = context.get("bar");
Map barMap;
if (UtilValidate.isEmpty(bar)) {
return;
} else if (bar instanceof String) {
barMap = parseStringAsJSONMap((String) bar);
} else if (bar instanceof Collection) {
barMap = convertCollectionToMap((Collection) bar);
} else if (bar instanceof Map) {
barMap = (Map) bar;
} else {
throw new IllegalArgumentException("Can't handle bar");
}
==
The first isEmpty call calls isEmpty(String). The latter calls
isEmpty(Object). The latter method then needs to do the instanceof
itself. But I assume you understand that.
Besides, you are deep into the 'instanceof always considered bad
camp'. This is wrong. See
http://www.velocityreviews.com/forums/t302491-instanceof-not-always-bad-the-instanceof-myth.html
Cheers,
Ruppert
On Nov 24, 2009, at 8:45 AM, Adam Heath wrote:
Adrian Crum wrote:
Or better yet, overload the method so you don't need all those
conditionals.
If the type of the object in the calling method is Object, then it
won't call the overloaded method.
However, this is better:
if (object instanceof String) {
return UtilValidate.isEmpty((String) object);
}
