Add ImmutableMultiset.copyOf(Iterable<Multiset.Entry<E>>)

[GoogleCodeExporter]

Currently I have code like this :

List<Multiset.Entry<String>> lefts = LdapUtils.transform(ldap.search(groupsDn, 
"(objectClass=groupOfUniqueNames)", SearchScope.ONELEVEL, "uniqueMember"), 
                                        ... );
ImmutableMultiset.Builder<String> builder = ImmutableMultiset.builder();
for (Multiset.Entry<String> left : lefts) {
    builder.addCopies(left.getElement(), left.getCount());
}
return builder.build();

it would be awesome if the last part can be shortened to just:

return ImmutableMultiset.copyOf(lefts);

Multiset already has a Multiset.Entry interface, however it is used just for 
output (Multiset.entrySet()) but I don't see it is used for input, which it 
would be very useful.

Original issue reported on code.google.com by ceefour666@gmail.com on 3 Nov 2012 at 3:32

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

It seems to me like it should be extremely rare that one would have an 
Iterable<Multiset.Entry> and not just a Multiset.

Original comment by cgdec...@gmail.com on 3 Nov 2012 at 7:13

• Added labels: Type-Addition

[GoogleCodeExporter]

This is similar to issue #320, but a different class. So I'm not the only one 
requesting this kind of addition, there is demand for this (not only to 
Multiset but across all collections in general).

Also, in the same idea, please also add the following APIs :

1. instance Multiset.addAll(Iterable<Multiset.Entry<E>>)
2. instance ImmutableMultiset.Builder.addAll(Iterable<Multiset.Entry<E>>)

Also similar to issue #679 but that one is more complex, and this one and issue 
#320 is actually trivial to implement. However doing this manually is 
unnecessary boilerplate code.

About "Multiset.Entry is rare", well in this case we don't yet have any 
Multiset instance, we're about to build one. Building a Multiset can be from 
various source, one I demonstrated (and is actually the code we use), is by 
providing a collection of Multiset.Entry objects.

This proposal is similar in nature to issue #320 (which is more popular and 
much older issue). It's also similar to ImmutableMap.of(K, V, ...) (why K, V? 
why not just Map? not everyone already has a Map at hand)

Original comment by ceefour666@gmail.com on 3 Nov 2012 at 7:30

[GoogleCodeExporter]

The question isn't implementation difficulty; it's a question of philosophy and 
API surface and whether or not we should actually be encouraging users towards 
this kind of code at all.

It still sounds like the only way to get a Multiset.Entry without having a 
Multiset in the first place is to have transformed a collection into 
Multiset.Entries.  How common is that?  How common *should* it be?  Might you 
have saved code by not using your LdapUtils.transform in the first place -- 
presumably saving you a Function that takes a minimum of five lines to write -- 
and just writing directly into the ImmutableMultiset.Builder in the traditional 
loop?

The reason this feature request isn't as compelling as it could be is that it 
still looks like this would only be useful if you had a transform() call that 
produced an Iterable<Multiset.Entry<E>> -- a transform call that might add more 
clutter than this feature would save.

Issue 320 has similar issues, which is why we similarly haven't decided whether 
or not to act on it yet.

Original comment by wasserman.louis on 3 Nov 2012 at 7:43

[GoogleCodeExporter]

One reason why transform can be more powerful than traditional loops is by 
utilizing concurrency. I'm not sure if Guava collections use any concurrency 
but with frameworks like Akka, it is always the case.

The transform function can be executed parallel by multiple threads, thereby 
increasing throughput of the resulting Iterable when the number of items is 
significant. (the assumption here is the transformation takes more time than 
the building of the Multiset/Multimap/etc.)

Another reason is passing a Builder instance isn't always possible or desirable.
In Actors model, only immutable objects should be passed between actors.
Passing Iterables directly is also good for fork-join.

Original comment by ceefour666@gmail.com on 3 Nov 2012 at 8:29

[GoogleCodeExporter]

Guava never introduces concurrency except in tools specifically designed for 
it, e.g. util.concurrent, because Guava works in environments where concurrency 
is strictly limited (AppEngine) or simply nonexistent (GWT).  Guava's built-in 
transforms tend to be implemented as views, though, which defers the 
computation to access time.  

That said -- building an ImmutableMap or ImmutableMultiset will always be 
sequential and linear time, so it's not clear to me that the building of the 
Multiset or Multimap would be slower than the transformation.

If you have a particularly expensive transformation function, then okay, I can 
understand the reasoning of preferring a custom parallelized transform and then 
putting entries into the multimap -- but that seems like a rare enough use case 
that it doesn't seem appropriate for specialized API support when perfectly 
good workarounds exist.  Most of the users we've seen inside Google who need to 
build a Multiset or a Map with "transformed entries" can transform their 
elements cheaply -- in which case they'd probably be better off using the 
straightforward for loop adding into the Builder, something along the lines of

ImmutableMultiset.Builder<E> builder = ImmutableMultiset.builder();
for (FooBar foobar : collection) {
  E element = ...
  int count = ...
  builder.addCopies(element, count);
}
ImmutableMultiset<E> multiset = builder.build();

...which doesn't involve passing around a Builder instance at all; it's only 
used as a local variable.

Original comment by wasserman.louis on 3 Nov 2012 at 8:46

[GoogleCodeExporter]

Indeed, the following lines are mandatory boilerplate :

ImmutableMultiset.Builder<E> builder = ImmutableMultiset.builder();
for (FooBar foobar : collection) {
  E element = ...
  int count = ...
  builder.addCopies(element, count);
}
ImmutableMultiset<E> multiset = builder.build();

"collection" here can be either Iterable<Multiset.Entry<E>> (using 
Multisets.immutableEntry()) or Map.Entry<E, Integer> (using 
Maps.immutableEntry()).
When we have tens of exactly same code to build a Multiset, a function to allow 
this instead of the above lines is very useful: (indeed we can implement this 
function ourselves for apps, but for libraries having this in Guava API is more 
convenient)

ImmutableMultiset<E> multiset = ImmutableMultiset.copyOf(colection);

Original comment by ceefour666@gmail.com on 4 Nov 2012 at 4:00

[GoogleCodeExporter]

Eh?  What we're arguing is that if you *have* an Iterable<Multiset.Entry<E>> or 
an Iterable<Map.Entry<E, Integer>>, how could you have gotten them in the first 
place?

1) You could have done a transform
   a) with a relatively heavyweight function -- which is pretty rare
   b) with a relatively lightweight function -- in which case you would've been better off using the Builder loop directly
2) You could have been simulating a Multiset yourself, e.g. with a Map<E, 
Integer> -- in which case you would've been better off using the Builder loop 
directly, or an intermediate mutable Multiset

From where we're sitting, most users who have an Iterable<Multiset.Entry<E>> or 
an Iterable<Map.Entry<E, Integer>> should have been using a Multiset in the 
first place, and should never have built that Iterable.  Those few users for 
whom it makes sense -- those users with heavyweight transforms that they might 
have parallelized -- have a straightforward workaround.

Looking at it another way -- you might have ten exact copies of the same 
builder loop, but how did you get the collections that were being passed into 
that builder loop?  Did you build ten different transform functions, for the 
ten different data structures you were transforming into Multisets?

Original comment by wasserman.louis on 4 Nov 2012 at 5:26

[GoogleCodeExporter]

Original comment by kak@google.com on 22 Aug 2013 at 11:17

• Changed state: Research

[GoogleCodeExporter]

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

• Added labels: MigratedToGitHub

[GoogleCodeExporter]

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

• Changed state: Migrated

[GoogleCodeExporter]

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

• Added labels: Restrict-AddIssueComment-Commit