DaveAKing / guava-libraries

Automatically exported from code.google.com/p/guava-libraries
Apache License 2.0
0 stars 0 forks source link

Add Futures.transform support for Iterable #1400

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Futures.transform currently allows transforming a single ListenableFuture. It 
would be handy if we could pass in an Iterable<ListenableFuture>, combining 
Iterables.transform and Futures.transform.

See attached file for a reference implementation. Methods that take an 
AsyncFunction instead of a regular Function could also be added.

Original issue reported on code.google.com by cgbeek...@gmail.com on 7 May 2013 at 10:21

Attachments:

GoogleCodeExporter commented 9 years ago
First, a quick acknowledgement: Your implementation uses Iterables.transform() 
with a heavyweight Function. As you've observed, every time that someone 
iterates over the resulting Iterable, the Function is applied anew.

This is part of the larger question of whether the transformation should be 
applied eagerly or lazily. Admittedly, Guava makes things more confusing than 
they ought to be: Iterables.transform is lazy; Futures.transform is eager. When 
the two collide, what should the user expect?

I think that the right way to go for a Futures method would be to apply the 
transformations eagerly. But that's all a question of implementation.

As for the question of whether to add this, I'll do some poking around in 
Google code to see if it seems to be coming up for many users, and I'll report 
back.

Original comment by cpov...@google.com on 7 May 2013 at 1:19

GoogleCodeExporter commented 9 years ago
I found 30-odd Iterable<ListenableFuture> references in the Google codebase. I 
found just one that was calling Futures.transform on each of its inputs, and it 
was (a) actually calling a custom variant of Futures.transform (b) also calling 
Futures.withFallback.

I'm kind of surprised that this isn't more common. Perhaps our internal 
CompletionTracker[*], which makes it easier to pull Futures off a list as they 
complete, is siphoning off some usage. We may get that promoted to Guava one 
day. Until then, you're probably best served with a for() loop. The Futures 
class is getting kind of big, so we like to see new methods do something 
hairier and more error-prone than that.

[*] similar to 
http://stackoverflow.com/questions/16309203/guava-listenablefuture-allaslist-ret
urns-all-content-seen-so-far-from-get#comment23373302_16309203

Original comment by cpov...@google.com on 7 May 2013 at 3:03

GoogleCodeExporter commented 9 years ago
Thanks for your extensive feedback, appreciated.

You are right about the eagerness of Future transformations, it makes much more 
sense to let Futures.transform behave the same for a single Future or multiple 
Futures in that respect.

Attached is a rather trivial implementation that simply bulk transforms the 
input Futures and returns them as a List. This prevents multiple 
transformations and fixes the eagerness "problem".

Even though it might be a trivial method, I still think inclusion in Futures is 
warranted, given that producing services are likely to return 
Iterable<ListenableFuture>. Without the method you would have to loop the 
futures, apply transformations to each individually and then put them in a List 
which is then passed to Futures.allAsList for example.

Regardless, CompletionTracker seems to be a handy utility to have. Would such 
an implementation allow returning Futures in order when desired? I.e. to create 
an aggregate that is order-sensitive? Without turning CompletionTracker into a 
full-blown fork/join implementation of course ;)

Original comment by cgbeek...@gmail.com on 8 May 2013 at 11:19

Attachments:

GoogleCodeExporter commented 9 years ago
How about this: We revisit this whenever we finally add AsyncLoadingCache 
(issue 1350), which is likely to have a getAll() method that returns Map<K, 
ListenableFuture<V>>.

For CompletionTracker, I've filed issue 1402. Can you post there about why 
you'd like to retrieve the Futures in order? (And of course add any other 
requirements you might have.)

Original comment by cpov...@google.com on 8 May 2013 at 3:30

GoogleCodeExporter commented 9 years ago
This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<issue id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:12

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:18

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:08