jamesbrowder / guava-libraries

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

Provide transformedCopy version of transform for Iterables and Lists #730

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
A couple times I have been bitten by using transform and having the processing 
intensive transformation action run multiple times. I know that the solution to 
this is to run the transform and then create a copy so that the copy can be 
used and the transformation action will only be done once. Seems like this is a 
common enough issue to include the following:

Lists:
   public static <T, U> List<U> transformedCopy(Iterable<T> from, Function<T, U> transform);

Iterables:
   public static <T, U> Iterable<U> transformedCopy(Iterable<T> from, Function<T, U> transform);

Original issue reported on code.google.com by dancerj...@gmail.com on 30 Sep 2011 at 4:57

GoogleCodeExporter commented 9 years ago
The big question here: what concrete class should transformedCopy() return? 
ArrayList? LinkedList? ImmutableList?

Also, would this "bloat" the API? Shouldn't we instead educate users and/or add 
rules to Findbugs to detect potentially dangerous uses of a transformed 
Iterable?

I could see this as being useful, but I'm also a fan of composing small utility 
methods together.

Another suggestion: have Iterables.transform() / Iterables.filter() return a 
FluentIterable interface that extends Iterable and adds methods such as 
toImmutableList(), toArrayList(), toLinkedList()... I think this might have 
been discussed before though.

Original comment by nev...@gmail.com on 30 Sep 2011 at 11:53

GoogleCodeExporter commented 9 years ago
I see your point. The API documentation does bring up this issue and suggest 
making a copy as appropriate. I like the idea of a different type / interface 
being returned (as  the suggestion of FluentIterable) that makes it clear that 
the method is returning a wrap around an existing object rather than a new 
object. This would make it very easy looking at the method signature to 
determine which are and are not doing lazy / wrapped "stuff". I could also see 
the point is this causing a large number of these types and bloating the API.

My main point is that I have been using Guava for a couple months now, use it 
daily and was really bit by this issue. I think an additional effort could be 
made to make this more clear but not sure what the best method is to do so 
given the issue of API bloat.

Original comment by dancerj...@gmail.com on 1 Oct 2011 at 2:41

GoogleCodeExporter commented 9 years ago
"My main point is that I have been using Guava for a couple months now, use it 
daily and was really bit by this issue. I think an additional effort could be 
made to make this more clear but not sure what the best method is to do so 
given the issue of API bloat."

I agree 100%.

I found a previous discussion of fluent iterables: 
http://code.google.com/p/guava-libraries/issues/detail?id=11 . Modifying 
Iterables.transform() and Iterables.filter() to return such fluent iterables 
would let users create copies in a clean manner, by chaining calls.

We'd still need to educate users, though, to make them aware that the returned 
iterable is a view / lazily-constructed...

Another solution might be to return an interface named "IterableView" (or 
"ViewIterable"), with a javadoc clearly explaining that this interface is a 
lazily-constructed view. The interface would provide methods such as 
toImmutableList(), toImmutableSet(), toArrayList()...

The method signature would then be self-explaining:

public static <F,T> IterableView<T> transform(Iterable<F> fromIterable, 
Function<? super F,? extends T> function)

Original comment by nev...@gmail.com on 1 Oct 2011 at 4:45

GoogleCodeExporter commented 9 years ago
I completely agree, I like the idea of returning a class in which copy methods 
are provided. I would really prefer to use method chaining rather than wrapping 
a call in a copy call. I feel the method chaining would be a lot more readable.

I also really like the idea of a ViewIterable. It is very clear from the method 
signature that you are getting a view and would allow for copying with method 
chaining.

Original comment by dancerj...@gmail.com on 1 Oct 2011 at 5:56

GoogleCodeExporter commented 9 years ago
I think Memoization can solve most the problems without chaning the Guava API 
drastically. The best implementation I found is:

Function<Integer, String> g = Functions.forMap(new 
MapMaker().makeComputingMap(f));

but this could be provided as Functions.memoize. (Suppliers.memoize is already 
implemented.) Your example transformedCopy becomes then:

Iterables.transform(Iterables.memoize(f));

Original comment by thomas.a...@gmail.com on 2 Oct 2011 at 12:27

GoogleCodeExporter commented 9 years ago
That seems a bit odd compared to just 
ImmutableList.copyOf(Iterables.transform(it, f)).

I think FluentIterable is probably the solution here (it does have 
.toImmutableSet and such), although it's not at all clear we would be 
*changing* the Iterables methods.  More likely you'd just stop using the 
Iterables class.

Original comment by kevinb@google.com on 2 Oct 2011 at 2:58

GoogleCodeExporter commented 9 years ago
I agree with Kevin, I think given what is currently available doing an 
Immutable copy is more readable. 

However, I still really like the idea of an IterableView which extends 
FluentIterable. Returning a class with "View" in the name makes it very clear 
what is going on. And having it extend Fluent would allow for very readable 
code. I would much prefer to write and read:

IterableView view = Iterables.transform(f);
List transform = view.immutableCopy();

- or -

List transform = Iterables.transform(f)
                          .immutableCopy();

Original comment by dancerj...@gmail.com on 3 Oct 2011 at 1:18

GoogleCodeExporter commented 9 years ago
It could even be argued that "FluentIterable" should be named "IterableView" 
instead. It's a little shorter, and documents that the transformed / filtered 
iterable is actually a view. Also, I'm not sure if it's even possible to end up 
with a iterable that is not a "view" when working with FluentIterable?

IterableView.of(myIterable)
              .transform(myFunction)
              .filter(mypredicate)
              .skip(3)
              .toImmutableList();

I remember reading a discussion a while ago about the difficulty of finding 
appropriate names for the fluent iterable / function / predicate constructs. 
Not sure if a consensus was reached on this.

Original comment by nev...@gmail.com on 3 Oct 2011 at 4:36

GoogleCodeExporter commented 9 years ago
Although I agree that having an IterableView that can do transforms and such, 
my starting point is that I think that the methods in Iterables that return a 
view should return an IterableView rather than a generic Iterable to give an 
indication that the returned element is a view. This would then facilitate 
using the toImmutableList, etc to create copies in a fluent manner.

Iterables.transform(myIterable, myFunction)
         .toImmutableList();

Original comment by dancerj...@gmail.com on 4 Oct 2011 at 3:39

GoogleCodeExporter commented 9 years ago
We continue to consider FluentIterable the appropriate solution here.

Original comment by fry@google.com on 5 Dec 2011 at 6:42

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

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

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

GoogleCodeExporter commented 9 years ago

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