Issue Details (XML | Word | Printable)

Key: COLLECTIONS-213
Type: Improvement Improvement
Status: Open Open
Priority: Major Major
Assignee: Unassigned
Reporter: Dusan Chromy
Votes: 1
Watchers: 2
Operations

If you were logged in you would be able to see more operations.
Commons Collections

CollectionUtils API extension: algorithm methods accept an Iterator argument

Created: 01/Jun/06 04:07 AM   Updated: 04/Nov/08 03:43 AM
Return to search
Component/s: Core
Affects Version/s: None
Fix Version/s: 4.x

Time Tracking:
Not Specified

File Attachments:
  Size
Text File Licensed for inclusion in ASF works COLLECTIONS-213.patch 2008-05-20 07:29 AM Henri Yandell 14 kB
Java Source File Licensed for inclusion in ASF works CollectionUtils.java 2006-06-01 04:09 AM Dusan Chromy 53 kB
Java Source File Licensed for inclusion in ASF works TestCollectionUtils.java 2006-06-01 04:10 AM Dusan Chromy 55 kB


 Description  « Hide
I just finished the Iterator additions to CollectionUtils, including test cases and I am going to
attach them to this issue very shortly (basically as soon as I figure out how attaching in JIRA works

At first I was thinking for a while whether CollectionUtils is a good place to accomodate the methods with the
new signature, until I noticed the collect method already accepts an Iterator argument.

Following methods now accept an Iterator argument (besides collect):

cardinality
find
forAllDo
countMatches
exists

I also noticed cardinality used to throw a NPE if the collection argument was null.
I see no reason why it should not return zero. The Iterator flavour does return zero
and I also modified the Collection version to return zero (including Javadoc
modification) for the sake of consistence.

I stopped to think for a while before touching the method, but the fact
that the Javadoc does not mention "may not be null" made me think the NPE is not thrown
intentionally. And after all, cardinality(..) is nothing else than specialized countMatches(..),
which returns 0 for null collection. However, feel free to reject the change to the cardinality(Object,Collection)
method if you think otherwise.

I worked on a fresh checkout from subversion and I just updated few minutes ago to make sure I have modified
the latest version. Anyway, please double-check before commiting the changes.

Cheers,

Dusan

> I think that these methods would make useful additions to the API.
>
> I don't have the time to do much collections work these days, but if you want to code the methods with test cases and attach them to JIRA then that would be great.
>
> Stephen
>
>
> dusan.chromy@freenet.de wrote:
>
> > I've been using some algorithm methods from the CollectionUtils, for example
> > find(Collection, Predicate)
> > exists(Collection, Predicate)
> > countMatches(Collection, Predicate)
> > forAllDo(Collection, Closure)
> >
> > However, I would also like to be able to use these algorithms with an Iterator:
> >
> > find(Iterator, Predicate)
> > exists(Iterator, Predicate)
> > countMatches(Iterator, Predicate)
> > forAllDo(Iterator, Closure)
> >
> > The obvious workaround is to use IteratorUtils.toList(Iterator), however this comes at the cost of constructing a list object (an ArrayList presumably) which could be avoided, as the Iterator itself is sufficient for the above algorithms to work.
> >
> > What do you think? Is there any reason not to provide the algorithms for an Iterator? I personally think that the algorithms should have been there for Iterators in the first place, because every collection is Iterable (or has an Iterator, prior to JDK 5.0).
> >
> > If noone is interested or has time to implement these changes, I can also contribute to the project - but at the moment I just wanted to discuss the idea / check if this has been already considered or planned.
> >
> > Best Regards,
> >
> > Dusan Chromy
>
>



 All   Comments   Work Log   Change History   Subversion Commits      Sort Order: Ascending order - Click to sort in descending order
Dusan Chromy added a comment - 01/Jun/06 04:09 AM
Addition of algorithm methods with an Iterator argument

Dusan Chromy added a comment - 01/Jun/06 04:10 AM
JUnit tests for newly added methods

Dusan Chromy added a comment - 16/Jun/06 10:49 PM
It just occured to me - in the middle of CollectionUtils-style programming - that the addition of the Iterator flavours methods will not only improve performance by avoiding the unnecessary conversion from iterator to List, but also add the following benefit:

Imagine you have an input collection of "raw" objects which you transform to something else by applying a Transformer. Now if you're only interested in finding a transformed object that satisfies a given predicate, having an Iterator flavour of find/exists method will allow you write a code like this:

Predicate predicate = ... ;
Transformer transformer = ... ;
CollectionUtils.find( IteratorUtils.transformedIterator( rawList.iterator(), transformer ), predicate );

This is going to perform better than the workaround:

CollectionUtils.find( CollectionUtils.collect( rawList, transformer ), predicate );

Because the latter needs to apply the transformer to all elements in the list, even if the first transformed element satisfies the predicate.


Stephen Kestle added a comment - 04/Feb/08 10:13 PM
BTW, CollectionUtils has been updated in the 4.0 branch to take Iterable as a parameter wherever possible.

But that CollectionUtils.find( CollectionUtils.collect( rawList, transformer ), predicate ); optimisation presents an interesting challenge to doing something similar with Iterables.


Matt Benson added a comment - 19/Mar/08 04:59 AM
Considering that IteratorUtils.toList(Iterator) is available, I don't see much value here and would vote WONTFIX. Dissent?

Ryan Ovrevik added a comment - 31/Mar/08 07:40 PM
Sorry for the delay in responding to the suggestion to close this
feature request.

Unless I am missing something, the functionality provided by creating
an intermediate list is not sufficient for our use cases. We use
iterators in a pipes and filters-based batch process. The iterators
are backed by fifo queues that are populated from database records.
Using the intermediate list approach would require that all items
piped through the process would first be read into (and stay in)
memory.


Henri Yandell added a comment - 20/May/08 07:29 AM
Attaching Dusan's changes as a patch as it's hard to work with old copies.