Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide transformedCopy version of transform for Iterables and Lists #730

Closed
gissuebot opened this issue Oct 31, 2014 · 10 comments
Closed

Comments

@gissuebot
Copy link

Original issue created by dancerjohn on 2011-09-30 at 04:57 PM


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);

@gissuebot
Copy link
Author

Original comment posted by neveue on 2011-09-30 at 11:53 PM


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.

@gissuebot
Copy link
Author

Original comment posted by dancerjohn on 2011-10-01 at 02:41 PM


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.

@gissuebot
Copy link
Author

Original comment posted by neveue on 2011-10-01 at 04:45 PM


"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: #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)

@gissuebot
Copy link
Author

Original comment posted by dancerjohn on 2011-10-01 at 05:56 PM


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.

@gissuebot
Copy link
Author

Original comment posted by thomas.andreas.jung on 2011-10-02 at 12:27 PM


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));

@gissuebot
Copy link
Author

Original comment posted by kevinb@google.com on 2011-10-02 at 02:58 PM


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.

@gissuebot
Copy link
Author

Original comment posted by dancerjohn on 2011-10-03 at 01:18 PM


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();

@gissuebot
Copy link
Author

Original comment posted by neveue on 2011-10-03 at 04:36 PM


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.

@gissuebot
Copy link
Author

Original comment posted by dancerjohn on 2011-10-04 at 03:39 PM


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();

@gissuebot
Copy link
Author

Original comment posted by fry@google.com on 2011-12-05 at 06:42 PM


We continue to consider FluentIterable the appropriate solution here.


Status: WontFix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant