commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bruno P. Kinoshita (Commented) (JIRA)" <>
Subject [jira] [Commented] (FUNCTOR-1) [functor] New components: summarize and aggregate
Date Thu, 22 Mar 2012 04:42:30 GMT


Bruno P. Kinoshita commented on FUNCTOR-1:

Hi Liviu! 

First of all, it's very clear you have put a lot of effort on this patch. I've applied at
a local working copy of functor trunk, and it is very neat indeed. I wish my patches were
as good as yours :-) 

I'm starting to learn about aggregators now, while reading the code and documents that you
wrote. So feel free to correct or comment any of my points below.

* The documentation and examples provided are very good :-) However, I think you used <br/>
to break line between paragraph, while the other examples use <p>. The visual is a bit

* After I read the issue and applied the patch (I had a quick view on it before, wow, so many
tests :) I opened Eclipse and looked for the aggregator package, but it's aggregate. I think
functor, generator and adapter package names follow the name of the pattern they implement.
And I have seen aggregate and aggregator as the name of the pattern. What do you think? 

* How does an aggregator differs from the reduce function (like in map reduce)? IIUC, aggregators
are like the fold function. A "nostore aggregator" is more like a fold with recursion, while
the list aggregator is a fold function which stores its partial value somewhere. Maybe we
could use FoldLeft with aggregators? 

* In the o.a.c.functor.aggregate.functions package, there are several aggregator functors
for Double and Integer. The name of aggregators for Double start with Double. But the name
of aggregators for Integer start with Int. In o.a.c.functor.generator.util, there are LongRange
and IntegerRange. So I think in this case we could put both as Int, or both as Integer. What
do you think?

* IIUC, in o.a.c.functor.aggregate there are classes for creating nostore and list aggregators.
However, if I create a ArrayListBackedAggregator<T>, this aggregator extends AbstractTimedAggregator<T>.
What means AbsractTimedAggregator<Integer> aggr = new ArrayListBackedAggregator<Integer>();
would be valid (not tested, but I think it works). I think these classes could be designed
in some other way to avoid this scenario... just food for thought :)

* AbstractTimedAggregator<T> implements TimedAggregator<T>. But in TimedAggregator#onTimer,
the first parameter is an AbstractTimedAggregator. I think this could be replaced by a TimedAggregator.
What do you think? Otherwise, it would be like the interface had a dependency to a class that
implements it. And if another class implemented TimedAggregator, it would have a dependency
to AbstractTimedAggregator too. Or if AbstractTimedAggregator was deprecated/removed, it would
cause problems in TimedAggregator interface.

The docs, examples and coverage are great. No issues in PMD, FindBugs or Checkstyle found
so far. All licenses in place, as well as package javadoc. Next time I submit a patch I hope
it can as neat as yours :-) 

There is an issue in Google Guava for similar feature, have you seen it (

> [functor] New components: summarize and aggregate
> -------------------------------------------------
>                 Key: FUNCTOR-1
>                 URL:
>             Project: Commons Functor
>          Issue Type: New Feature
>         Environment: JDK 1.6.0_25 but should work with any JDK 5+ (possibly 1.4 though
I haven't tested).
>            Reporter: Liviu Tudor
>            Assignee: Simone Tripodi
>            Priority: Minor
>              Labels: features
>         Attachments:, commons-functor.patch.bz2,
functor.patch.bz2, functor.patch.bz2
> This is the next step from -- as instructed
I'm finally hoping to get the code in the right place and hopefully this is something that
the functor component could do with.
> Whereas initially I just started with the summarizer component, I have added now the
second one, the "aggregator" as they are somehow related. If this code proves to be useful
to functor in any way, it would actually be good to get some feedback on these 2 to see if
the class hierarchy can in fact be changed to share some common functionality as I feel (probably
due to the similar needs that lead to writing/using these components) that somehow they should
share a common base.
> In brief, the 2 components:
> * aggregator: this just allows for data to be aggregated in a user defined way (e.g.
stored in a list for the purpose of averaging, computing the arithmetic median etc). The classes
provided actually offer the implementation for storing data in a list and computing the above-mentioned
values or summing up everything.
> * timed summarizer: this is another variation of the aggreator, however, it adds the
idea of regular "flushes", so based on a timer it will reset the value and start summing/aggregating
the data again. Rather than using an aggregator which would store the whole data series (possibly
for applying more complex formulas), this component just computes on the fly on each request
the formula and stores the result of it. (Which does mean things like computing arithmetic
mean, median etc would be difficult to compute without knowing upfront how many calls will
be received -- i.e. how many elements we will be required to summarize/aggregate.) So the
memory footprint of running this is much smaller -- even though, as I said, it achieves similar
results. I have only provided a summarizer which operates on integers, but obviously others
for float, double etc can be created if we go ahead with this design.
> Hopefully the above make sense; this code has resulted from finding myself writing similar
components to these a few times and because it's always been either one type (e.g. aggregator)
or another (summarizer) I haven't given quite possibly enough thought to the class design
to join these 2. Also, unfortunately, the time I sat down to make these components a bit more
general and submitted issue 340 was nearly 3 months ago so I'm trying to remember myself all
the ideas I had at a time so bear with me please if these are still  a bit fuzzy :) However,
if you can make use of these I'm quite happy to elaborate on areas that are unclear and obviously
put some effort into getting these components to the standards required to put these into
a release.

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:!default.jspa
For more information on JIRA, see:


View raw message