Iterables.transform() should cache transformed values

[GoogleCodeExporter]

It would be much more performant if the first time iterating through a
transform the values were cached, so that on subsequent iterations the
objects can just be returned directly rather than a bunch of new ones created.

Original issue reported on code.google.com by rwallace...@gmail.com on 9 Jun 2009 at 10:12

[GoogleCodeExporter]

Making this type of caching a default behavior would be a gross 
overoptimization.
Most functions are not so slow that this is a win, nor does a little extra 
instance
creation typically cost you much.

But if you plan to iterate several times and you do want to make sure the 
function
isn't re-evaluated every time, you can always copy the result into a new 
collection
first:

  ImmutableList.copyOf(Iterables.transform(list, function))

Original comment by kevin...@gmail.com on 9 Jun 2009 at 10:47

• Changed state: WontFix

[GoogleCodeExporter]

This strikes me as a bad idea.

If the contents of fromIterable are mutable, then caching may simply be 
incorrect -- 
since it's not possible to ensure that no changes take place between iterations 
over 
the transformed version.  I think this could result in some very frustrating 
debugging sessions.  It arguably violates the principle of least surprise, but 
that's probably largely dependent on your specific programming background.

You could also just implement a caching function for your transform, and get 
the 
same benefit, but with much, much more control over the details:

/**
 * F should be immutable for the life of the function.
 */
abstract class MemoFn implements Function<F,T> {

   /** Cache for the results of MemoFns **/
   private final Map<F, T> _memos = Maps.newHashMap();

   public T apply(F from) {
      T result = _memos.get(from);

      if (null == result) {
         result = memoApply(from);
         _memos.put(from, result);
      }
      return result;
   }   

   public abstract T memoApply(F from);
}

Original comment by cresw...@gmail.com on 9 Jun 2009 at 10:49

[GoogleCodeExporter]

Ah, the caching function wrapper is a good idea that I hadn't thought of.  Good 
call,
that's definitely a better way to go.  Thanks!

Original comment by rwallace...@gmail.com on 9 Jun 2009 at 11:13

[GoogleCodeExporter]

Maybe instead we could create a new utility method in Functions: 

    Function<F, T> Functions.cache(Function<F, T> func)

And add javadocs that if the Iterable being transformed isn't immutable (or 
otherwise
thread-safe) or the type <F> isn't safe, then bad things could happen?

Original comment by rwallace...@gmail.com on 9 Jun 2009 at 11:16

[GoogleCodeExporter]

I've added a request (and patch, if anyone wants to play with it immediately) 
for 
Functions.memoize(...).  See issue 190.

Original comment by cresw...@gmail.com on 11 Jun 2009 at 10:09