tikinghu / google-collections

Automatically exported from code.google.com/p/google-collections
0 stars 0 forks source link

Be consistent with [Collection]s.new*[Collection](...) #304

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I recently tried to change a Sets.newHashSet(E...) to a
Sets.newLinkedHashSet(E...) but I couldn't, since it simply doesn't exist.

Is it possible to add the following factory methods (not taking in account
the issue 113):
Sets.newLinkedHashSet(E...);
Sets.newLinkedHashSet(Iterator<? extends E>);
Sets.newEnumSet(Iterator<E>,Class<E>);
Sets.newEnumSet(Class<E>, E...);
Sets.newTreeSet(E...);
Sets.newTreeSet(Comparator<? super E>, E...);
Sets.newTreeSet(Comparator<? super E>, Iterable<? extends E>);
Sets.newTreeSet(Iterator<? extends E>);
Sets.newTreeSet(Comparator<? super E>, Iterator<? extends E>);

Lists.newLinkedList(E...);
Lists.newLinkedList(Iterator<E>);

Maps seem consistent enough, but this might be interesting:
Maps.newHashMap(Map.Entry<K,V>...);
Maps.newHashMap(Iterator<Map.Entry<K,V>>);
Maps.newHashMap(Iterable<Map.Entry<K,V>>);
etc.

Original issue reported on code.google.com by ogregoire on 4 Dec 2009 at 1:09

GoogleCodeExporter commented 9 years ago
I'd absolutely LOVE to be consistent on this -- by removing the varargs forms 
of 
Lists.newArrayList() and Sets.newHashSet()!

These methods should never have been added. There's almost always a better way! 
Most 
often, BY FAR, the user really is better served with ImmutableList.of(...). In 
the 
other cases, we are certainly within the body of a method, so one can simply do 
this:

  Set<Foo> foos = Sets.newLinkedHashSet();
  Collections.addAll(foos, a, b, c);

... or if a one-liner is really preferred:

  import static java.util.Arrays.asList;

  Set<Foo> foos = Sets.newLinkedHashSet(asList(a, b, c));

I strongly endorse the habit of statically importing Arrays.asList() as your 
general-
purpose "turn varargs into a collection" workaround. (If that gives you 
unchecked 
warnings, switching to ImmutableList.of() sometimes fixes that.)

With Iterators, you have similar options. This in particular is just not a 
common 
enough need to warrant all these methods in our API.

Anyway, removing the varargs methods Lists.newArrayList(E...) and 
Sets.newHashSet(E...) to provide the "consistency" you desire is not feasible 
due to 
the overwhelming volume of callers they have already. I've reviewed these and I 
believe we'd all be better off if every last one of them changed to one of the 
other 
forms, but it's too much effort to actually do all that now.

Original comment by kevinb@google.com on 8 Dec 2009 at 5:41

GoogleCodeExporter commented 9 years ago
Why am I better served with ImmutableList.of(...)?  In what way is 

Set<Foo> foos = Sets.newLinkedHashSet(a, b, c);

inferior to the other options presented here?  It seems to me that 
newLinkedHashSet with var args takes up fewer characters and is thus more 
concise, easier to read, and faster to type.

The reason why we have libraries, frameworks and programming languages is to 
offer us pieces of pre-built functionality that make coding faster and less 
verbose so we can keep our code concise and focused on business logic.

The only possible argument I can come up with for not wanting the var-args 
stuff inside Google Guava is that it introduces code duplication within the 
Guava library.  So if the duplicate code isn't in Guava, that moves the 
duplicate code into callers of Guava.  That is putting the duplicate code in 
many many more places than if it was inside Guava.

If we follow this logic to its logical conclusion, we should do away with 
garbage collection, then Java and virtual machines, then C and just program 
everything in assembly language.

Original comment by matthew....@gmail.com on 10 May 2011 at 3:38

GoogleCodeExporter commented 9 years ago
This project is no longer maintained. I linked here as example of arguments for 
refusal of going further. Please continue discussion on Guava's page, but... 
good luck!

Original comment by ogregoire on 10 May 2011 at 7:23