Review our collections which accept backing collections from the user

[GoogleCodeExporter]

(Need to enumerate this list.)

A "backing collection" means a collection that the user supplies for its
behavior/performance qualities but which we expect them never to refer to
directly again.  As contrasted with "view" methods like Lists.transform()
where it is acceptable for client code to refer to both the wrapped and the
wrapping instances.

We need to review these and:

* convince ourselves that it is worth continuing this support (note that
Bob argues strongly for removing this from ReferenceMap)

* for those that remain, we must remove the type parameters from the
accepted collection and accept a RAW collection, which we will then cast to
what we need. If we don't, we expose an implementation detail, and would be
unable to change a multiset implementation, for example, to use
AtomicInteger or Integer or a custom class. We have no reason not to retain
this flexibility except for a knee-jerk reaction against raw types.

* for those that remain, we should probably make sure we are always
checking that the collection is empty (even though this doesn't catch all
possible violations by a long shot, it will catch some).

* for those that remain, we must standardize on very strongly-worded
documentation on the ills of retaining any reference to the backing collection.

Original issue reported on code.google.com by kevin...@gmail.com on 6 Jun 2008 at 7:00

[GoogleCodeExporter]

Here's a list of such collections in the library.

ConcurrentMultiset(ConcurrentMap<E,Integer>) constructor

Multimaps.newMultimap(), newListMultimap(), newSetMultimap(), 
newSortedSetMultimap()

ReferenceMap(ReferenceType keyReferenceType, ReferenceType valueReferenceType,
ConcurrentMap<Object,Object> backingMap) constructor

I left out cases like Comparators.givenOrder(), 
Multisets.unmodifiableMultiset(),
Multimaps.forMap(), Sets.union(), and the Forwarding* classes, for which this 
issue's
concerns don't apply. We care about the situations when the code sometimes uses 
a
user-provided backing collection and sometimes doesn't.

Original comment by jared.l....@gmail.com on 22 Jul 2008 at 7:37

[GoogleCodeExporter]

Great, thanks.  Bob is determined that we remove that RefMap constructor.  I 
also
think we can lose the ConcurrentMultiset one.  I'll leave it to you to decide 
what to
do for the Multimaps ones.

Original comment by kevin...@gmail.com on 22 Jul 2008 at 7:41

[GoogleCodeExporter]

I'd rather keep the new*Multimap methods. since the benefits of creating a 
multimap
with arbitrary backing collections are more compelling.

However, I'm fine with getting rid of the other two. The most probable reasons 
for
wanting an explicit ConcurrentMap are
  1) specifying the ConcurrentHashMap initialCapacity/loadFactor/concurrencyLevel
  2) specifying a ConcurrentSkipListMap for a deterministic sort order

If those features are requested, we could create factory methods that take those
parameters explicitly. Besides, a ConcurrentSkipListMap by itself wouldn't be 
enough
for a sorted ReferenceMap with weak or soft references, since the ReferenceMap
implementation doesn't store the keys directly in the map.

Original comment by jared.l....@gmail.com on 22 Jul 2008 at 8:27

[GoogleCodeExporter]

Original comment by kevin...@gmail.com on 17 Sep 2009 at 5:45

• Added labels: Milestone-1.0

[GoogleCodeExporter]

Original comment by kevin...@gmail.com on 17 Sep 2009 at 5:46

• Removed labels: 1.0

[GoogleCodeExporter]

This issue now refers only to the Multimaps.new*Multimap() methods.  I've 
reread all 
the stuff I said, and thought about it and I think these methods will be all 
right the 
way the are. I'm closing this.

Original comment by kevin...@gmail.com on 18 Sep 2009 at 4:51

• Changed state: Fixed