Standardize how to create instances of our collections.

[GoogleCodeExporter]

It's a bit of a mess out there right now.

All our concrete collection implementation classes should contain their own
static creation methods, rather than having them lumped into classes like
Lists and Sets.  (Lists and Sets would continue to contain creation methods
only for JDK collection types; I'll file a separate issue to discuss those.)

For each of our implementation classes:

I propose that we keep constructors to a minimum, possibly having any
constructors besides the parameterless one being private.  Then I propose
that we have the following static creation methods:

1. newInstance() with no parameters - create an empty instance
2. newInstance(Iterable<? extends E>) (or Map, etc. for other non-Iterable
types) - the "copy constructor"
3. newInstance(....) accepting sizing parameters such as expectedSize, if
applicable
4. newInstanceBackedWith(...) which accepts a backing collection instance,
if applicable

We will NOT have:

* newInstance(Iterator) (use Iterables.addAll())
* newInstance(E[]) or newInstance(E...) (use Arrays.asList() or
Collections.addAll())
* Any combinations of 1-4 above (e.g., initial data plus *also* sizing
parameters or backing collection).

Note that it is important to name #4 above differently from the rest
because we have several implementations that both implement Map and are
backed with a Map.

I also propose that our interfaces provide some documentation that
encourages all implementations to follow these conventions.

Feedback?

Original issue reported on code.google.com by kevin...@gmail.com on 31 Oct 2007 at 6:18

[GoogleCodeExporter]

Yay for minimal constructors.  Yay for not supporting varargs creation.

One taste question (danger: bikeshed discussion looming) about the static 
creation
methods; how do we feel about more evocative names than "newInstance"?  You say 
that
it is important to name #4 differently, but might it be desirable to name #3
differently?  Or #2?

  SnazzyMap.withBlort(37);
  SnazzyMap.fromMap(fooMap);

Original comment by hilsd...@gmail.com on 31 Oct 2007 at 9:05

[GoogleCodeExporter]

In general, this seems like a good idea.  however, you should make sure that 
you do
not screw up things people may want to do via reflection (for introspective 
apis and
the like).  obviously you should have a public noargs constructor, but the copy
constructor is also one i could see reflective code depending on.

Original comment by jahlborn@gmail.com on 1 Nov 2007 at 2:12

[GoogleCodeExporter]

No, I believe it's highly illegitimate for any code to depend on the presence 
of a
particular public constructor beyond the parameterless one.

Original comment by kevin...@gmail.com on 1 Nov 2007 at 9:55

[GoogleCodeExporter]

hilsdale: on the naming issue, I like your suggestions. It would be nice to 
know what
is going to end up happening in the JDK; if there is going to be
HashMap.newInstance() then we may as well just get with that program now.  ONE 
of the
places where this JDK issue is tracked is here:

http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=5092426

Original comment by kevin...@gmail.com on 1 Nov 2007 at 11:22

[GoogleCodeExporter]

I agree with hilsdale on naming.  Methods with names like "newInstance" should 
be
used only when we genuinely cannot come up with something more meaningful (and I
suspect this will be rare).

Original comment by cbif...@gmail.com on 12 Mar 2008 at 6:17

[GoogleCodeExporter]

Original comment by kevin...@gmail.com on 27 May 2008 at 6:30

• Added labels: 1.0

[GoogleCodeExporter]

First just let me say, fantastic framework. 

Now my question is, why are there not "new" methods for the Multisets class? For
example I can do something like the following to create a new HashSet:

Set<String> stringSet = Sets.newHashSet();

but I can't do something like the following to create a new Multiset:

Multiset<String> stringBag = Multisets.newHashMultiset();

instead I have to use the factory method in HashMultiset called create() to get 
a new
instance of a HashMultiset. Seems like you should be consistent throughout your 
api,
e.g. either use create() methods or "new" methods. I prefer the "new" methods 
but I'm
sure you can juggle the pros and cons. 

Thanks for your time and again, fantastic work!

Original comment by Matthew....@gmail.com on 17 Feb 2009 at 12:37

[GoogleCodeExporter]

This question of where to locate all the factory methods has two main solutions 
and
both are correct.  We had to choose one.  We decided to consistently put the 
methods
on the implementation class whenever possible (ie, when it's our own 
implementation
class), and fall back on kitchen-sink utility classes like Sets only when 
necessary
(because it's a JDK implementation class).

Original comment by kev...@google.com on 26 Feb 2009 at 5:00

[GoogleCodeExporter]

Issue 112 has been merged into this issue.

Original comment by kev...@google.com on 26 Feb 2009 at 5:01

[GoogleCodeExporter]

Kevin, by this rule of consistency it follows that ArrayListMultimap, 
LinkedListMultimap etc, should have had a create() method instead of falling 
back to 
Multimaps utility methods. So, can you elaborate a bit on this particular 
apparent 
discrepancy?

Original comment by jim.andreou on 26 Feb 2009 at 5:43

[GoogleCodeExporter]

Fixing the Multimap factory methods has been on my to-do list for a while, but 
I've
been busy with other stuff. We'll deal with it before the next release.

Original comment by jared.l....@gmail.com on 26 Feb 2009 at 6:28

[GoogleCodeExporter]

We are sooooo close now.  Just need CustomFooMultimap.create() to replace
Multimaps.newFooMultimap() and I think we're done!!

Original comment by kevin...@gmail.com on 24 Mar 2009 at 10:09

[GoogleCodeExporter]

The LinkedListMultimap API docs have an incorrect example for instantiation.  
The
class header still shows the use of a standard constructor, when that 
constructor is
not publicly visible.

http://google-collections.googlecode.com/svn/trunk/javadoc/index.html?http://goo
gle-collections.googlecode.com/svn/trunk/javadoc/com/google/common/collect/packa
ge-summary.html

{{{
Multimap<K, V> multimap = new LinkedListMultimap<K, V>();
}}}

Original comment by cresw...@gmail.com on 8 Apr 2009 at 6:37

[GoogleCodeExporter]

Thanks, I'll fix that.

Original comment by kevin...@gmail.com on 8 Apr 2009 at 7:34

[GoogleCodeExporter]

I like the factory methods.  One case where I am having difficulty is with 
synchronized wrappers.  E.g. I'd like to create a synchronized SetMultimap in 
one 
line.

This would be great, but compiler complains:

private final SetMultimap<String, String> map = 
    Multimaps.synchronizedSetMultimap(HashMultimap.create());

I have to make in intermediate reference to the unsynchronized map:

private <K, V> SetMultimap<K, V> createSynchronizedSetMultimap() {
    SetMultimap<K, V> unsynchronized = HashMultimap.create();
    return Multimaps.synchronizedSetMultimap(unsynchronized);
}

Original comment by will.h...@gmail.com on 14 Apr 2009 at 1:04

[GoogleCodeExporter]

The following single-line command does what you want.

private final SetMultimap<String, String> map = 
    Multimaps.synchronizedSetMultimap(HashMultimap.<String, String>create());

The generics-after-the-dot syntax looks a little strange, but it works.

Original comment by jared.l....@gmail.com on 14 Apr 2009 at 1:29

[GoogleCodeExporter]

The lack of a no-arg constructor for classes like HashMultimap is a serious 
problem 
for any use of reflection. Its a colossal pain if you're using hibernate, for 
example. Following the constructor pattern of the standard collections would 
make far 
more sense than static factory methods.

Original comment by csurri...@informatica.com on 4 May 2009 at 10:30

[GoogleCodeExporter]

I agree with Chris Surridge.  This just hit me with HashMultiset.  This began 
with
Kevin saying, "I propose that we keep constructors to a minimum, possibly 
having any
constructors besides the parameterless one being private." But how did that 
turn into
"no public constructors at all", not even parameterless?

Original comment by matthew....@gatech.edu on 27 Jun 2009 at 12:34

[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]

The request for constructors was filed in issue 160.

The work described in this bug record is complete except for 
Multimaps.new*Multimap(). 
We have decided to just live with this inconsistency. In the future, it's 
likely those 
methods will become deprecated, and everyone will have maybe two years in which 
to get 
their code fixed to the newer, better API.

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

• Changed state: Fixed