views:

85

answers:

4

I'm using a third party library that returns a raw Iterator, e.g.

Iterator<?> children = element.getChildElements();

I know the actual type, but I don't necessarily trust the third party lib to stick with it in the future. There are two (that I can think of) somewhat risky ways to traverse this:

@SuppressWarnings("unchecked")
Iterator<ActualObject> currentChildren = (Iterator<ActualObject>)currentElement.getChildElements();

or

Iterator<?> children = element.getChildElements();
while (null != children && children.hasNext()) {
  ActualObject child = (ActualObject)children.next(); //Possible ClassCastException @ runtime
  ...
}

The only "safe" way I could come up with to traverse this sort of iterator is the following:

Iterator<?> children = element.getChildElements();
while (null != children && children.hasNext()) {
  Object obj = children.next();
  ActualObject child = null;
    if (obj instanceof ActualObject)
      child = (ActualObject)obj;
    ...
}

This seems overly verbose. Is there a better, yet equally "safe" way to traverse a raw iterator?

EDIT: I realize I can catch/log the exception in an else block, I was looking (hoping) for a Java language equivalent of what ColinD has mentioned below.

+5  A: 

Guava makes this easy with its Iterators.filter(Iterator<?>, Class<T>) method. It returns an unmodifiable Iterator<T> that basically just skips every element of the given iterator that isn't an instance of type T:

Iterator<ActualObject> children = Iterators.filter(element.getChildElements(),
    ActualObject.class);

You can obviously then traverse the resulting iterator without needing to cast each element to ActualObject AND without having to worry about ClassCastException.

ColinD
@ColinD - Thanks, that is interesting, but unfortunately using Guava isn't an option for me.
Segphault
@Segphault: well then read the source code and check how they did it.
seanizer
I suspect they did the same thing as your last example, but just extracted it into a wrapper object.
pkaeding
Yes, it does effectively do the same thing as the second example, though it builds on the rest of their predicate/filtering code.
ColinD
Segphault
+2  A: 

The easy way to deal with it is just cast it and make sure that, if the type changes from what you expect in the future, an exception will be thrown and you'll be notified. You will want to check out that however you're logging exceptions works and the exception will find its way to the log file. Exceptions exist in order to tell you something isn't what you expect, let them do their job.

If you silently skip elements that are not the expected type then you could be missing out on data you need, that doesn't sound like a good solution to me.

Nathan Hughes
I agree with the second part
seanizer
+2  A: 

Your last version seems like the way to go but I guess we can improve that a bit:

Iterator<?> children = element.getChildElements();
// no null check needed. If an api that supposedly
// returns an iterator actually returns null, it's a bad
// API, don't use it
while (children.hasNext()) {
    Object obj = children.next();
    if (obj instanceof ActualObject)
        doStuffWith((ActualObject)obj);
    // we know it's of the right type so we might
    // as well put the cast in the method call.
}

So it boils down to this:

Iterator<?> children = element.getChildElements();
while (children.hasNext()) {
    Object obj = children.next();
    if (obj instanceof ActualObject)
        doStuffWith((ActualObject)obj);
}

which I'd say isn't too bad.

Edit:

There should probably be an else block underneath the if block, too. Something along the lines of:

else{
    log.warn("Expected type: " + ActualObject.class + ", but got " + obj);
}
seanizer
You're right about the null check, I'll remove that. It IS a bad API, in more ways than evidenced by the question. I can't wait to never use it again.
Segphault
+1  A: 

Since it sounds like you want to stop execution and throw an exception if you encounter an object returned by the iterator that is not an instance of ActualObject, then I would just cast it, and have a catch block to deal with the possible ClassCastException.

pkaeding