yf0994 / guava-libraries

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

Transform an Iterable<T> to a Multimap<K, V> with a Function<T, K> and a Function<T, V> #1151

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Hi,

I hit a use-case where I have to create a Multimap from a Collection by 
applying the functions to each value to produce keys as well as values. For 
Example:

class Person {
  String name = ...;
  Integer birthYear = ...;
}

Collection<Person> persons = ...;

Function personNameFunction = ...;
Function personBirthYearFunction = ...;

Multimap<Integer, String> index =
   Multimaps.index(persons, personNameFunction, personBirthYearFunction);

Will, may be 'index' is a bad name here and should be 'transformFrom'.

Here is implementation which I can think of (and use in my code for now) :

public static <K, V, T> Multimap<K, V> index(Iterable<T> values,
              Function<T, K> keyFunction, Function<T, V> valueFunction) {
      checkNotNull(keyFunction);
      checkNotNull(valueFunction);
      ImmutableListMultimap.Builder<K, V> builder = ImmutableListMultimap.builder();
      Iterator<T> valueIterrator = values.iterator();
      while (valueIterrator.hasNext()) {
        T value = valueIterrator.next();
        checkNotNull(value, values);
        builder.put(keyFunction.apply(value), valueFunction.apply(value));
      }
      return builder.build();
 }

The complete patch attached.

Thanks a lot
   kofemann.

Original issue reported on code.google.com by kofem...@gmail.com on 18 Sep 2012 at 1:55

GoogleCodeExporter commented 9 years ago
How does this not work?

Multimap namesByBirthYear = Multimaps.transformValues(
  Multimaps.index(persons, personToBirthYearFunction),
  personToNameFunction);

Original comment by ogregoire on 18 Sep 2012 at 4:28

GoogleCodeExporter commented 9 years ago
@ogregoire: that's a very nice approach!

That said, I'm not fully convinced that the OP's suggestion actually improves 
readability over the direct approach without any helper methods:

ImmutableListMultimap.Builder<K, V> builder = ImmutableListMultimap.builder();
for (Person p : people) {
  builder.put(p.getName(), p.getBirthYear());
}
return builder.build();

Original comment by wasserman.louis on 18 Sep 2012 at 6:05

GoogleCodeExporter commented 9 years ago
@wasserman.louis: this works as long as you have simple getXxx(). With more 
complicated function, simple loop is not elegant any more.

@ogregoire: sure, this will work as well and looks like preferred Guava way, as 
there is no transformEntries() for Map to change type of the key.

Original comment by kofem...@gmail.com on 18 Sep 2012 at 7:51

GoogleCodeExporter commented 9 years ago
@kofemann: If a Function can do what you want, you can always just write a 
method that will keep it equally elegant.

builder.put(complicatedFunction(p), anotherFunction(p));

Original comment by cgdec...@gmail.com on 18 Sep 2012 at 8:51

GoogleCodeExporter commented 9 years ago
Yep.  You can always factor that out into its own method -- and in most cases I 
can think of where the logic is that complicated, you *should.*

Original comment by wasserman.louis on 18 Sep 2012 at 8:57

GoogleCodeExporter commented 9 years ago
Right, and here you see the difference between

ImmutableListMultimap.Builder<K, V> builder = ImmutableListMultimap.builder();
for (Person p : people) {
  builder.put(complicatedFunction(p), anotherFunction(p));
}
return builder.build();

and 

return Multimaps.index(persons, complicatedFunction, anotherFunction);

Anyway, you can clone the issue.

Original comment by kofem...@gmail.com on 18 Sep 2012 at 9:06

GoogleCodeExporter commented 9 years ago
Yes, that is the right side-by-side comparison, but I'm not sure option 2 is 
*necessarily* better than option 1.  "Multimaps.index" doesn't help me figure 
out what that call is actually doing, whereas the builder approach makes it 
very obvious exactly what you're doing there.

Original comment by wasserman.louis on 18 Sep 2012 at 9:23

GoogleCodeExporter commented 9 years ago

Original comment by lowas...@google.com on 14 Aug 2013 at 10:09

GoogleCodeExporter commented 9 years ago
We like the alternatives in comment #1 and #2 instead of adding a new API.

Original comment by kak@google.com on 22 Aug 2013 at 10:21

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:13

GoogleCodeExporter commented 9 years ago

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