Feature request: interface (CollectionBuilder<T>) that ImmutableList.Builder<T> implements for generic building

[GoogleCodeExporter]

I am implementing set of serializers and deserializers for handling collection 
types. While existing builder system is convenient when explicitly creating 
instances from code, it makes reuse of building functionality very difficult.

This is due to all various Builder inner classes being unrelated physical 
types, even though conceptually there are but couple of logical abstractions 
(collection builder, map builder).

A simple way to improve this would be to create 2 new interfaces; something 
like:

public interface CollectionBuilder<T> {
  public void add(T... elements);
  public void add(T element); // and so forth
  // and optionally:
  // public CT build();
}

// and similarly MapBuilder<K,B> { }

And make existing Builder inner classes implement said interfaces.

My specific use case is one where during deserialization I create a builder 
(either with direct code or via reflection, given collection type), deserialize 
elements, add, and build at the end.
Most of code is fully generic (just pass element value deserializer), but 
currently I need to cut'n paste most of the code as builder types are distinct.

Original issue reported on code.google.com by tsaloranta@gmail.com on 29 Nov 2010 at 2:20

• Merged into: #396

[GoogleCodeExporter]

We'll consider this, but I'll note that we have never been comfortable with the 
idea of treating these Builders as first-class types suitable for passing 
around across APIs.

Original comment by kevinb@google.com on 8 Dec 2010 at 3:28

[GoogleCodeExporter]

Thanks. I understand hesitation, but it would be very very convenient for data 
binding toolkits (and bit more generally for generators that constructs lists 
and maps).
Anyway I appreciate your consideration.

Original comment by tsaloranta@gmail.com on 8 Dec 2010 at 4:53

[GoogleCodeExporter]

I still feel that the type you actually pass around should be a more general 
sort of Receiver, not a collection builder.

Original comment by kevinb@google.com on 12 Jan 2011 at 10:38

• Added labels: Type-Enhancement

[GoogleCodeExporter]

I am fine with other abstractions; for deserialization it just has to be 
something that allows adding elements to collection being built, and in generic 
fashion.
I just suggested something that would be obvious: but it might not be best fit 
with library design.

Also: if there is some part of functionality that makes sense to use instead of 
a new feature I would be interested.

Original comment by tsaloranta@gmail.com on 13 Jan 2011 at 12:02

[GoogleCodeExporter]

Just to see if I understand what you're looking for...

Here's my current understanding:

ImmutableList<String> deserialize(Input input) {
  return deserialize(new ImmutableList.Builder<String>(), input);
}

ImmutableSet<String> deserialize(Input input) {
  return deserialize(new ImmutableSet.Builder<String>(), input);
}

<C extends Collection> C deserialize(CollectionBuilder<C> builder, Input input) 
{ ... }

If it's that simple, I'd probably do it this way:

ImmutableList<String> deserialize(Input input) {
  return ImmutableList.copyOf(deserialize(input));
}

ImmutableSet<String> deserialize(Input input) {
  return ImmutableSet.copyOf(deserialize(input));
}

List deserialize(Input input) { ... }

I could take a couple guesses as to why you'd want the generic builder 
(avoiding copying for non-Immutable* classes, <something> where it's important 
to abstract the various static copyOf() methods behind an interface), but what 
do you have in mind?

Original comment by cpov...@google.com on 13 Jan 2011 at 12:49

[GoogleCodeExporter]

Ideally I would want to avoid creating unnecessary copies. But it is true that 
creating a temporary list would allow reusing part that collects entries to 
include.

Original comment by tsaloranta@gmail.com on 13 Jan 2011 at 3:16

[GoogleCodeExporter]

The current implementations of ImmutableList.build and ImmutableSet.build 
maintain an ArrayList of elements and implement build() as "return 
copyOf(contents)."  For ImmutableSet, I'd expect that to continue to be the 
case forever; for ImmutableList, it's conceivable that we could be more clever 
in the future, but this seems unlikely because we have to be careful to 
maintain immutability.

IMO there are only three reasons to use the Builder classes over ArrayList and 
friends:

1) You want to construct a static constant (so you need to stay within a single 
statement) and you can't do it with of()/copyOf().

2) You're building a Map, where the Builder's entries ArrayList may have 
performance advantages over a full-on HashMap.

3) The name "Builder" clarifies what you're using the Builder/ArrayList for.  
(Of course, you can name the variable "builder" regardless of what you call the 
class.

I do see that you brought up ImmutableMap, so (2) could definitely apply here.  
(And of course it's conceivable that the builders will improve in some other 
way in the future.)  I still don't know that this will be a high priority, but 
it's good to know that people are interested.  If anyone else has other use 
cases, let us know.

Original comment by cpov...@google.com on 13 Jan 2011 at 3:59

[GoogleCodeExporter]

Ah. I did not check out implementation -- given that a copy is made anyway, 
there isn't much to optimize (unless implementation of ImmutableList and -Set 
change).
This is good to know. I can see why this is bit different for Maps. I plan to 
support both (ideally, most or all guava types, but the plug-in for java types 
is work-in-progress and I'll start with types I mostly use myself).

I agree with seeing if others find this useful: even for me it is a 
nice-to-have, given how things are. Thank you for explanation and help here.

Original comment by tsaloranta@gmail.com on 13 Jan 2011 at 6:05

[GoogleCodeExporter]

Original comment by kevinb@google.com on 26 Jan 2011 at 2:23

• Changed state: Accepted
• Added labels: Priority-Low

[GoogleCodeExporter]

Hey, this is very similar to issue 396.

http://code.google.com/p/guava-libraries/issues/detail?id=396

Original comment by ray.j.gr...@gmail.com on 26 Jan 2011 at 8:57

[GoogleCodeExporter]

True; too bad I didn't notice it originally. Could have just augmented the 
other issue.

Original comment by tsaloranta@gmail.com on 27 Jan 2011 at 6:38

[GoogleCodeExporter]

other issue hereby augmented.

Original comment by kevinb@google.com on 27 Jan 2011 at 1:15

• Changed state: Duplicate

[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:15

• Added labels: MigratedToGitHub

[GoogleCodeExporter]

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

• Added labels: Restrict-AddIssueComment-Commit