ImmutableCollection.copyOf()

[GoogleCodeExporter]

Guava provides ImmutableList.copyOf() and ImmutableSet.copyOf() but no 
ImmutableCollection.copyOf().

Use-case: A class constructor accepts a "Collection" which I'd like to make a 
defensive copy of. If the collection is already an ImmutableList/Set/etc then 
return "this", otherwise make a copy and return a new object.

Original issue reported on code.google.com by cow...@bbs.darktech.org on 14 Dec 2012 at 7:39

[GoogleCodeExporter]

Perhaps it is better to place such a method on ImmutableList/Set/etc so users 
get to decide what implementation should be used in case a copy needs to be 
made.

How about copyAsCollection()?

Original comment by cow...@bbs.darktech.org on 14 Dec 2012 at 7:49

[GoogleCodeExporter]

If you want to make a defensive copy of an input collection, you just use 
ImmutableList/Set.copyOf(). If you don't care exactly what kind of collection 
you get, just use ImmutableList.copyOf(). If the input is an 
ImmutableCollection, it will use the asList() view of that collection if 
possible... and even better, the asList() view of an ImmutableSet will retain 
the O(1) contains() complexity.

Original comment by cgdec...@gmail.com on 14 Dec 2012 at 8:18

• Changed state: WorkingAsIntended
• Added labels: Package-Collect, Type-Addition

[GoogleCodeExporter]

This does not address the following use-case:

1. User passes an ImmutableSet into the constructor.
2. The constructor would like to share a copy of the ImmutableCollection if 
possible but if a copy must be made then it would prefer an ImmutableList.

If I understand correctly, your approach would cause needless conversion from 
ImmutableSet to ImmutableList.

Original comment by cow...@bbs.darktech.org on 14 Dec 2012 at 8:22

[GoogleCodeExporter]

The ImmutableSet has an array of elements. Its asList() view uses the same 
array with no copying. So while ImmutableList.copyOf(immutableSet) doesn't 
return the input ImmutableSet directly, it returns a thin wrapper around it, 
and that wrapper is even cached so that if multiple calls to asList() or 
ImmutableList.copyOf(immutableSet) are made, they all get the same 
ImmutableList object. So while it's "converting" from ImmutableSet to 
ImmutableList, the actual work involved is minimal.

Original comment by cgdec...@gmail.com on 14 Dec 2012 at 9:00

[GoogleCodeExporter]

Yup.  The work done by ImmutableList.copyOf(ImmutableSet) is tiny and constant, 
and more or less amounts to one object allocation.

Original comment by lowas...@google.com on 14 Dec 2012 at 9:02

[GoogleCodeExporter]

I have no problem with thin wrapper layers so long as you can guarantee that 
the contents won't get copied even if multiple permutations are used over time. 
For example: ImmutableList.copyOf(ImmutableSet.copyOf(ImmutableList))

1. Can you make such a guarantee for today's implementations?
2. Can you add this requirement to the specification, to ensure that all future 
sub-classes follow it?

Original comment by cow...@bbs.darktech.org on 15 Dec 2012 at 2:51

[GoogleCodeExporter]

That chain of queries would inevitably make a copy no matter how fancy the 
implementation.

If you're asking whether we can make ImmutableSet.copyOf(ImmutableSet.asList()) 
avoid a copy...we'll see what we can do.

Original comment by wasserman.louis on 15 Dec 2012 at 2:55

[GoogleCodeExporter]

Louis,

That's my point... if you were to add copyAsCollection() you *could* guarantee 
no copies would take place, for all sub-classes of ImmutableCollection.

Original comment by cow...@bbs.darktech.org on 15 Dec 2012 at 3:05

[GoogleCodeExporter]

How realistic is this use case, though?  That this is actually 
performance-critical?

Original comment by wasserman.louis on 16 Dec 2012 at 1:00

[GoogleCodeExporter]

This is first and foremost a design restriction. Right now I'm forced to accept 
Set or List as input because I cannot safely handle immutable Collections. I 
can't control how big or small the input collection will be so I can't comment 
on the kind of performance I will see out in the wild.

Original comment by cow...@bbs.darktech.org on 16 Dec 2012 at 1:40

[GoogleCodeExporter]

I feel obliged to note that this wouldn't be terribly difficult to implement 
yourself:

if (c instanceof ImmutableCollection) {
  return c;
} else {
  return ImmutableList.copyOf(c);
}

Original comment by wasserman.louis on 16 Dec 2012 at 1:51

[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 3 Nov 2014 at 9:08

• Added labels: Restrict-AddIssueComment-Commit