A filtered iteration over a collection: current idiom?

S

Simon Brooke

I'm looking for the most idiomatic and elegant means of iterating over a
filtered subset of a collection. Here's the basic structure of piece of
code I'm looking at, which must be fairly common:

Vector<Widget> widgets = doSomethingToGetWidgets();

for (Widget widget : widgets) {
if (widget instanceof ActionWidget) {
doSomethingWithActionWidget( (ActionWidget) widget);
}
}

(obviously, ActionWidget is a subclass of Widget)

What I'd like to do would be something like

Vector<Widget> widgets = doSomethingToGetWidgets();

for (ActionWidget widget : widgets
where (widget instanceof ActionWidget)) {
doSomethingWithActionWidget( (ActionWidget) widget);
}

I can't find anything in the Java 5 collections documentation which
offers type filtering functionality; am I missing something?
 
J

Jean-Baptiste Nizet

Le 18/09/2010 16:36, Simon Brooke a écrit :
I'm looking for the most idiomatic and elegant means of iterating over a
filtered subset of a collection. Here's the basic structure of piece of
code I'm looking at, which must be fairly common:

Vector<Widget> widgets = doSomethingToGetWidgets();

for (Widget widget : widgets) {
if (widget instanceof ActionWidget) {
doSomethingWithActionWidget( (ActionWidget) widget);
}
}

(obviously, ActionWidget is a subclass of Widget)

What I'd like to do would be something like

Vector<Widget> widgets = doSomethingToGetWidgets();

for (ActionWidget widget : widgets
where (widget instanceof ActionWidget)) {
doSomethingWithActionWidget( (ActionWidget) widget);
}

I can't find anything in the Java 5 collections documentation which
offers type filtering functionality; am I missing something?

I personally find the code very easy to read and understand as is, but
you might be interested in apache commons collections :
http://commons.apache.org/collections/

The code could be rewritten as is :

List<Widget> widgets = doSomethingToGetWidgets();
CollectionUtils.forAllDo(widgets, ClosureUtils.ifClosure(
PredicateUtils.instanceOfPredicate(ActionWidget.class),
new Closure() {
@Override
public void execute(Object o) {
doSomethingWithActionWidget((ActionWidget) o);
}
}));

(not compiled, not tested)

Please stop using Vector. List/ArrayList should be preferred.

JB.
 
L

Lew

I'm looking for the most idiomatic and elegant means of iterating over a
filtered subset of a collection. Here's the basic structure of piece of
code I'm looking at, which must be fairly common:

Vector<Widget> widgets = doSomethingToGetWidgets();

for (Widget widget : widgets) {
if (widget instanceof ActionWidget) {
doSomethingWithActionWidget( (ActionWidget) widget);
}
}

(obviously, ActionWidget is a subclass of Widget)

What I'd like to do would be something like

Vector<Widget> widgets = doSomethingToGetWidgets();

Wha...??? Vector? Really? Come on! You're just yanking our chain, right?

No, really, 'fess up. You're pulling our leg, aren't you?

Aren't you?
for (ActionWidget widget : widgets
where (widget instanceof ActionWidget)) {
doSomethingWithActionWidget( (ActionWidget) widget);
}

I can't find anything in the Java 5 collections documentation which
offers type filtering functionality; am I missing something?

Yeah, that what you did there is an antipattern. Use proper object
orientation and the problem magically melts away.

Instead of 'doSomethingWith( Foo foo )' implement 'Foo.doSomething()'. Then
you get type-based execution as a proper concomitant to polymorphism. That/s
the whole freaking *POINT* of object-orientation, for Pete's sake!

for( Widget widget : somehowGetWidgets() )
{
widget.doSomething();
}

Then 'ActionWidget' subclass instances will do the
'ActionWidget#doSomething()' override and 'PassionWidget' subclass instances
will do the 'PassionWidget#doSomething()' override, each doing the right thing
for its own type automagically without silly 'instanceof' tests.

If you really need your iteration to happen only over 'ActionWidget' instances
there really isn't anything inbuilt in Java to do what you asked for without
an explicit 'if ( widget instanceof ActionWidget )' test, but the very
presence of that test is a red flag that you got your object model wrong.

If you don't have a 'Collection <ActionWidget>' in the first place your
problem is upstream.
 
M

Mike Schilling

Lew said:
Wha...??? Vector? Really? Come on! You're just yanking our chain,
right?

No, really, 'fess up. You're pulling our leg, aren't you?

Aren't you?


Yeah, that what you did there is an antipattern. Use proper object
orientation and the problem magically melts away.

Instead of 'doSomethingWith( Foo foo )' implement 'Foo.doSomething()'.
Then you get type-based execution as a proper concomitant to polymorphism.
That/s the whole freaking *POINT* of object-orientation, for Pete's sake!

for( Widget widget : somehowGetWidgets() )
{
widget.doSomething();
}

Then 'ActionWidget' subclass instances will do the
'ActionWidget#doSomething()' override and 'PassionWidget' subclass
instances will do the 'PassionWidget#doSomething()' override, each doing
the right thing for its own type automagically without silly 'instanceof'
tests.

If you really need your iteration to happen only over 'ActionWidget'
instances there really isn't anything inbuilt in Java to do what you asked
for without an explicit 'if ( widget instanceof ActionWidget )' test, but
the very presence of that test is a red flag that you got your object
model wrong.

If you don't have a 'Collection <ActionWidget>' in the first place your
problem is upstream.

I don't entirely agree. Suppose you have a collection of widgets of
different types, some of which require explicit actions when disposed, and
some of which do not.

for (Widget widget : widgetCollection)
{
if (widget instanceof Disposable)
{
((Disposable)widget).dispose();
}
}

seems quite reasonable to me. Yes, you could make all Widgets implement a
no-op dispose() method, but that becomes more onerous as the number of such
optional features increases, unless all Widgets derive from a common base
class.

And just for (a sufficiently warped version of) fun, here's a class that
enables iterating over a type-filtered collection:

import java.util.Arrays;
import java.util.Iterator;
import java.util.List;
import java.util.NoSuchElementException;


public class FilteredCollection<T, C> implements Iterable<C>
{
private Iterable<T> collection;
private Class<C> filter;

private FilteredCollection(Iterable<T> collection, Class<C> filter)
{
this.collection = collection;
this.filter = filter;
}

public static <T, C> Iterable<C> getFilteredCollection(Iterable<T>
collection, Class<C> filter)
{
return new FilteredCollection<T, C>(collection, filter);
}

public Iterator<C> iterator()
{
return new FilteredIterator();
}

private class FilteredIterator implements Iterator<C>
{
private Iterator<T> iterator;
private C nextObject;

private FilteredIterator()
{
iterator = collection.iterator();
fill();
}

public boolean hasNext()
{
return nextObject != null;
}

public C next()
{
if (nextObject == null)
{
throw new NoSuchElementException();
}
C retval = nextObject;
fill();
return retval;
}

public void remove()
{
throw new UnsupportedOperationException();
}

protected void fill()
{
while (iterator.hasNext())
{
T next = iterator.next();
if (filter.isInstance(next))
{
nextObject = filter.cast(next);
return;
}
}
nextObject = null;
}
}

public static void main(String[] args)
{
List<?> objects = Arrays.asList(1, 2.2, "a", null, false, "b");
for (String s : getFilteredCollection(objects, String.class))
{
System.out.println(s);
}
}
}
 
L

Lew

Mike said:
And just for (a sufficiently warped version of) fun,
here's a class that enables iterating over a type-filtered collection:

Brilliant. Bravo.

You show how to put type safety back into what would otherwise be too risky.

Reflective type tricks work best when buried in type-safe utility classes like
this.

Applause also for "when the API don't got it, yer roll yer own as if 'twere in
the API." Very Java-like code, a perfect example of type-oriented programming.

Good API writing and good example of maintainability, too.

Good show.
import java.util.Arrays;
import java.util.Iterator;
import java.util.List;
import java.util.NoSuchElementException;

Single-type imports, a best practice.
public class FilteredCollection<T, C> implements Iterable<C>

'Iterable' is the least-most broad type applicable, a best practice.

("Least-most broad" - types should be the broadest applicable but no broader
than strictly necessary. A generalization of "prefer interface types".)

Implementation of a standard API type is a best practice.

'Iterable' is a great workhorse type.

Shouldn't that class definition be
{
private Iterable<T> collection;
private Class<C> filter;

Classic run-time type token.
private FilteredCollection(Iterable<T> collection, Class<C> filter)
{
this.collection = collection;
this.filter = filter;
}

public static <T, C> Iterable<C> getFilteredCollection(
Iterable<T> collection, Class<C> filter )

Just to remind the public of a gotcha - the type parameters of a static method
are not the same as those of the generic class type itself, which are bound to
instances. The 'T' and 'C' in this static method could have equivalently been
'U' and 'F':

public static <U, F extends U>
Iterable <F> getFilteredCollection(
Iterable <U> collection, Class <F> filter )

It's a beautiful thing how this declaration supports type inference for teh
clients:

Iterable <ActionWidget> actionWidgets = getFilteredCollection(
getAllWidgets(), ActionWidget.class );

where 'getAllWidgets()' returns a collection (rather, iterable) of some
supertype of 'ActionWidget'.

Mike's example uses type inference to make a compact 'for-each' expression.
{
return new FilteredCollection<T, C>(collection, filter);
}

public Iterator<C> iterator()
{
return new FilteredIterator();
}

private class FilteredIterator implements Iterator<C>
{
private Iterator<T> iterator;
private C nextObject;

private FilteredIterator()
{
iterator = collection.iterator();
fill();
}

public boolean hasNext()
{
return nextObject != null;
}

public C next()
{
if (nextObject == null)
{
throw new NoSuchElementException();
}
C retval = nextObject;
fill();
return retval;
}

public void remove()
{
throw new UnsupportedOperationException();
}

protected void fill()
{
while (iterator.hasNext())
{
T next = iterator.next();
if (filter.isInstance(next))

Run-time type token is better and safer than 'instanceof' chains.
{
nextObject = filter.cast(next);

Also better than hard-coded casts.
return;
}
}
nextObject = null;
}
}

public static void main(String[] args)
{
List<?> objects = Arrays.asList(1, 2.2, "a", null, false, "b");
for (String s : getFilteredCollection(objects, String.class))

There's that slick use of type inference.
{
System.out.println(s);
}
}
}

Thanks, Mike.
 
A

Arne Vajhøj

I don't entirely agree. Suppose you have a collection of widgets of
different types, some of which require explicit actions when disposed,
and some of which do not.

for (Widget widget : widgetCollection)
{
if (widget instanceof Disposable)
{
((Disposable)widget).dispose();
}
}

seems quite reasonable to me. Yes, you could make all Widgets implement
a no-op dispose() method, but that becomes more onerous as the number of
such optional features increases, unless all Widgets derive from a
common base class.

Stuff like that is frequently seen in real world Java.

I will claim that is almost all cases, then the object
model could have been designed better to avoid the need.

But in the real world we are usually stuck with the
object model once created forever.

Arne
 
L

Lew

What if the something in the OP's doSomethingWith method has greater
knowledge and context within the larger system, which widgets are not
privy to?

doSomethingWithWidget(Widget foo) is not necessarily == widget.doSomething()

Good question. If the 'doSomething()' method relies on the specific subtype
of its 'Widget' argument to do the right thing, then it is in turn not taking
advantage of polymorphism or method overloading correctly.

Proper type analysis would yield a 'doSomething( Widget foo )' method that
needs to know only what the argument type reveals. i.e., 'Widget' attributes
and behavior, and leaves subtype details to the subtypes. If it needs to know
more than that the argument is 'Widget', then the argument is mis-declared.

The outer method should never have to reach in to the innards of the
implementation of its arguments like that, per the Law of Demeter.
 
M

Mike Schilling

Shouldn't that class definition be
public class FilteredCollection <T, C extends T> implements Iterable<C>

Not necessarily. C might be an interface that's unrelated to T, as in my
initial example of Disposable.

Anyway, thanks very much for all the kind words.
 
L

Lew

Lew wrote ...
Mike said:
Not necessarily. C might be an interface that's unrelated to T, as in my
initial example of Disposable. [repeated here:]
for (Widget widget : widgetCollection)
{
if (widget instanceof Disposable)
{
((Disposable)widget).dispose();
}
}

Very instructive. Multiple inheritance Java-style haiku.

E.g.,

Action Serializable
\\ //
\\ //
Widget Disposable
\\ //
\\ //
DisposableWidget

You have excellent command of the interfaces.
 
R

Roedy Green

Instead of 'doSomethingWith( Foo foo )' implement 'Foo.doSomething()'. Then
you get type-based execution as a proper concomitant to polymorphism. That/s
the whole freaking *POINT* of object-orientation, for Pete's sake!

Sometimes that logic is not really part of the object. Then you might
want to implement a delegate and use the Visitor pattern.
 
L

Lew

Lew wrote, quoted or indirectly quoted someone who said :
Roedy said:
Sometimes that logic is not really part of the object. Then you might
want to implement a delegate and use the Visitor pattern.

True but not pertinent to my point, from which you excerpted out the context.

My point is that dependency on the object's type is not one of those
"sometimes". Labeling something by a GoF pattern doesn't automatically make
it correct or best practice, and if your "Visitor pattern" is keying off the
subtype of an argument then the odds are very, very high that you are doing it
wrong.

Others have pointed out corner cases where you bend that rule, but it's
important to bear in mind that they are corner cases. Nearly all the time you
have an "instanceof" test you would do better to use real object orientation.

Rules of thumb are only meant to apply most of the time, of course, but they
are meant to be considered every time.
 
D

Daniel Pitts

Stuff like that is frequently seen in real world Java.

I will claim that is almost all cases, then the object
model could have been designed better to avoid the need.

But in the real world we are usually stuck with the
object model once created forever.
I almost agree.
Young projects change easily.
Older projects (6 mo after first launch maybe?) change with slight effort.
Older yet projects (1 year after first launch?) change with modest effort.

Refactoring larger/older projects takes effort, especially if the
architecture itself needs to be rethought, however it is not impossible.
The difficulty comes from the following dilemma: I have 1 week to
implement a 6 new features. To do it "cleanly" will take 2 weeks, to do
it with an "if instance of" will take 3 days.

My boss was pleased it only took 3 days, so doubled the feature requests
for next week. No time to go back and fix the code.

This dilemma can be mitigated by managing expectations. "I can get a
throw-away prototype of this feature out in 3 days, but it will need to
be re-written after we launch it."

Anyway, type safe filtering Iterable is a useful utility.
 
A

Arne Vajhøj

Refactoring larger/older projects takes effort, especially if the
architecture itself needs to be rethought, however it is not impossible.
The difficulty comes from the following dilemma: I have 1 week to
implement a 6 new features. To do it "cleanly" will take 2 weeks, to do
it with an "if instance of" will take 3 days.

My boss was pleased it only took 3 days, so doubled the feature requests
for next week. No time to go back and fix the code.

He is not the only one ...

Arne
 
S

Simon Brooke

Wha...??? Vector? Really? Come on! You're just yanking our chain,
right?

I'm working with legacy libraries. So shoot me. Some of us live in the
real world.
 
L

Lew

Simon said:
I'm working with legacy libraries. So shoot me. Some of us live in the
real world.

That's some legacy, going back to 1998 and all.

So shoot me - some of us face the real world instead of perpetrating crap that
should have been retired before we were out of short pants.

You're using generics, so your code is not old enough to use the lame "legacy"
excuse.

"Real world" indeed. Pffth. Large bull dropping load.
 
E

Eric Sosman

That's some legacy, going back to 1998 and all.

So shoot me - some of us face the real world instead of perpetrating
crap that should have been retired before we were out of short pants.

You're using generics, so your code is not old enough to use the lame
"legacy" excuse.

"Real world" indeed. Pffth. Large bull dropping load.

1998 was twelve whole years ago. Nothing that old can possibly
be of any relevance today. Therefore, Lew is (1) less than twelve
years old, or (2) irrelevant.

Or (3) dropping a large load.
 
A

Arne Vajhøj

That's some legacy, going back to 1998 and all.

So shoot me - some of us face the real world instead of perpetrating
crap that should have been retired before we were out of short pants.

You're using generics, so your code is not old enough to use the lame
"legacy" excuse.

"Real world" indeed. Pffth. Large bull dropping load.

One of the reasons behind the implementation of generics
those for Java is that new code works with old libraries,
so the fact that he is using generics proves nothing.

Arne
 
A

Arne Vajhøj

I'm working with legacy libraries. So shoot me. Some of us live in the
real world.

It is seen before.

I will note though that such a library is starting to become a risk
in itself.

Arne
 
L

Lew

Eric said:
1998 was twelve whole years ago. Nothing that old can possibly
be of any relevance today. Therefore, Lew is (1) less than twelve
years old, or (2) irrelevant.

Or (3) dropping a large load.

It's #3.
 

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,764
Messages
2,569,566
Members
45,041
Latest member
RomeoFarnh

Latest Threads

Top