Making a general copy of a Set in constructor

Question

I did not find a direct answer to this specific question, so... Assume class:

class MyClass {
    private final Set<String> tags;

    MyClass(Set<String> initialTags) {
        this.tags = ???(initialtags);
    }
}

I just want a copy without caring about exact implementation of the set, with minimal overhead, so basically a direct copy of whatever internal state initialTags has. I can't use clone, since Set is interface. Is this possible, or do I have to use something like new HashSet<String>(tags);, needing to make a decision about the type?

Assumptions: Set item type is resticted to immutable. I can trust the caller to not pass a broken initialTags Set implementation, and I can trust it to have good enough performance for any internal use in this class.

I'm thinking of trying reflection and clone(), but this seems a bit dirty...

1) You can never trust the caller :); 2) I don't see anything wrong with `new HashSet(tags);` unless you have a benchmark to demonstrate that it doesn't perform well (I'm assuming you'll have less than 10 items in the set). — Augusto, Nov 02 '12 at 09:27
what do you mean by "a broken initialTags Set implementation"? If you are sure that the elements of the Set are immutable then why can't the client pass and unmodifiable set? — Aravind Yarram, Nov 02 '12 at 09:35
*"with minimal overhead"* => what is the typical size of your sets? — assylias, Nov 02 '12 at 09:35
Yeah, deciding the type is probably what I'll do in the code that prompted me to write this question, instead of some ugly reflection hack, it's certainly "good enough". This question is a bit more general, "is there a way to do this". Apparently answer is "no". — hyde, Nov 02 '12 at 09:39
You could try `initalTags.getClass().newInstance() ... .addAll()` but there's no guarantee the `set` implementation being passed will have a no-arg constructor. — John Dvorak, Nov 02 '12 at 09:41

score 4 · Accepted Answer · answered Nov 02 '12 at 09:39

Guava provides a method for that:

  /**
   * Returns an immutable set containing the given elements, in order. Repeated
   * occurrences of an element (according to {@link Object#equals}) after the
   * first are ignored. This method iterates over {@code elements} at most once.
   *
   * <p>Note that if {@code s} is a {@code Set<String>}, then {@code
   * ImmutableSet.copyOf(s)} returns an {@code ImmutableSet<String>} containing
   * each of the strings in {@code s}, while {@code ImmutableSet.of(s)} returns
   * a {@code ImmutableSet<Set<String>>} containing one element (the given set
   * itself).
   *
   * <p>Despite the method name, this method attempts to avoid actually copying
   * the data when it is safe to do so. The exact circumstances under which a
   * copy will or will not be performed are undocumented and subject to change.
   *
   * @throws NullPointerException if any of {@code elements} is null
   */
  public static <E> ImmutableSet<E> copyOf(Iterable<? extends E> elements) {
    return (elements instanceof Collection)
        ? copyOf(Collections2.cast(elements))
        : copyOf(elements.iterator());
  }

score 1 · Answer 2 · answered Nov 02 '12 at 10:05

Depending on what the expected size of initialTags is you may prefer different implementations; the code below shows quicker times for constructor-based copies when i is < 100000, but clone is faster when i > 1000000. If you really want to use clone you could always use isinstance to determine which of the (6) well known concrete implementations (that implement cloneable) it is, beyond that you have no guarantee initialTags has it.

static Set<String> byCloning(HashSet<String> initialTags) {

    return (Set<String>) initialTags.clone();
}

static Set<String> byConstructor(Set<String> initialTags) {

    return new HashSet<String>(initialTags);
}

public static void main(String[] args) {

    int N = Integer.parseInt(args[0]);

    HashSet<String> mySet = new HashSet<String>();
    for (int n = 0 ; n < N ; n++) mySet.add(Integer.toString(n));

    long start = System.currentTimeMillis();
    byCloning(mySet);
    long end = System.currentTimeMillis();
    System.out.println(" time take in milli seconds for cloning = " + (end-start) );

    start = System.currentTimeMillis();
    byConstructor(mySet);
    end = System.currentTimeMillis();
    System.out.println(" time take in milli seconds by constructor = " + (end-start) );
}

Interesting benchmarks, thanks, but the question was specifically about Set<> without specifying the exact implementation. — hyde, Nov 02 '12 at 10:26
sorry, I was making the point that to make the copy you should use a concrete implementation (since you need to specify how your copy is stored anyway). Only certain Set implementations allow `clone` so without doing some 'dirty' reflection tricks... — robert, Nov 02 '12 at 10:30

score 0 · Answer 3 · answered Nov 02 '12 at 09:31

0

Either make a decision on the type based on the kind of access you need of the set, or you could try using Collections.unmodifiableSet() :

class MyClass {
    private final Set<String> tags;

    MyClass(Set<String> initialTags) {
        this.tags = Collections.unmodifiableSet(initialTags);
    }
}

answered Nov 02 '12 at 09:31

Andy MacGilvery

11
1

4

`unmodifiableSet` _won't_ guarantee the set won't be modified, only that it won't be modified through the `unmodifiableSet` adapter (which is not very useful in this case). – John Dvorak Nov 02 '12 at 09:32

Making a general copy of a Set in constructor

3 Answers3