1
\$\begingroup\$

Below is my working code written as an exercise in wildcards in generics and Java 8 type inference. My question will be about the commented line in union() method.

I needed to create an instance of the interface type that does not include a static factory, so I was left with (seemingly) two choices:

  1. Use concrete implementation (as in my code below)
  2. Use Reflection to retrieve the actual run-time class and call its constructor (similar to my earlier question).

I chose to use the first approach and used a constructor of a concrete class HashSet<E> (that implements Set). I picked HashSet among other publicly available implementations rather arbitrarily.

My reasoning was that any would suffice, since it will be returned by a method as just Set<E> and, though it will retain its HashSet-specific members, they will not interfere with a client process.

Was my decision safe and sound or is there a legitimate way to break my method code?

import java.util.*;


public class TypeInference {

    public static <E> Set<E> union (Set<? extends E> s1, Set<? extends E> s2) {
        Set<E> result = new HashSet<E>();  // or any other concrete Set
        result.addAll(s1);
        result.addAll(s2);
        return result;
    }

    public static void main(String[] args) {

        Set<Integer> ints = new TreeSet<>(Arrays.asList(1, 2, 3, 4, 5));
        Set<Double> doubles = new TreeSet<>(Arrays.asList(1.0, 2.0, 3.0, 4.0, 5.0));

        Set<Number> numbers = union(ints, doubles);
        System.out.println(numbers);
    }

}
\$\endgroup\$
  • 1
    \$\begingroup\$ Things may or may not go wrong depending on the contract of your method. If you explained what it is supposed to do precisely we would be able to answer your Q. For example: is union supposed to return a set of the same subtype as s1? as s2? any set? should it throw an exception if s1 and s2 don't have the same type? Is the method supposed to retain the original order (if there is one)? Or work based on iteration order over s1 and s2? etc. Once you know exactly what the method needs to do, answering your question will be easy. \$\endgroup\$ – assylias May 26 '15 at 18:29
  • \$\begingroup\$ Actually the main purpose of the method was to verify that Java type inference has been sufficiently enhanced so such code no longer produces an incompatible types error (as it did at some point according to Josh Bloch's talk (around 31:23). \$\endgroup\$ – PM 77-1 May 26 '15 at 19:36
  • \$\begingroup\$ That error he mentions is a compile time error that you sometimes got before Java 8 due to the weak-ish type inference system. Java 8 has immensely improved on that front. But in any case it (being a compile error) means that if your code compiles then it is fine (and in fact it should compile with Java 8 and Java 7). \$\endgroup\$ – assylias May 26 '15 at 20:20
  • \$\begingroup\$ In order for my code to compile without warnings I had to find a way initialize result. And this is what my question is about. The rest was just the explanation why I wrote all this code. \$\endgroup\$ – PM 77-1 May 26 '15 at 22:40
1
\$\begingroup\$

This is as safe as your Set implementation for result. For example, swapping for a TreeSet gave the following error:

Exception in thread "main" java.lang.ClassCastException: 
            java.lang.Integer cannot be cast to java.lang.Double
    at java.lang.Double.compareTo(Double.java:49)
    at java.util.TreeMap.put(TreeMap.java:568)
    at java.util.TreeSet.add(TreeSet.java:255)
    at java.util.AbstractCollection.addAll(AbstractCollection.java:344)
    at java.util.TreeSet.addAll(TreeSet.java:312)

This is understandable since TreeSet imposes an ordering, which in turns calls the to-be-added Collection-type's compareTo() with the existing entries. This has nothing to do with type incompatibility at compile-time (the code compiles), rather it's a limitation/feature of the TreeSet implementation.

In your linked question @rolfl suggested giving a Supplier to the method, which I will advocate here too:

public class TypeInference {

    public static <E> Set<E> union (Set<? extends E> s1, Set<? extends E> s2, 
            Supplier<Set<E>> supplier) {
        Set<E> result = supplier.get();
        result.addAll(s1);
        result.addAll(s2);
        return result;
    }

    public static <E> Set<E> union (Set<? extends E> s1, Set<? extends E> s2) {
        return union(s1, s2, () -> new TreeSet<>(Comparator.comparing(Object::toString)));
    }

    public static void main(String[] args) {
        Set<Integer> ints = new TreeSet<>(Arrays.asList(1, 2, 3, 4, 5));
        Set<Double> doubles = new TreeSet<>(Arrays.asList(1.0, 2.0, 3.0, 4.0, 5.0));
        System.out.println(union(ints, doubles));
        System.out.println(union(ints, doubles, 
                () -> new TreeSet<>(Comparator.comparing(Number::doubleValue))));
    }
}

The two-argument union() uses a TreeSet that compares by the entries' toString(), which lets us store both Integer and Double values together in the Set. Alternatively, calling union() with a TreeSet supplier that compares by Number.doubleValue() gives us only 5 distinct elements:

[1, 1.0, 2, 2.0, 3, 3.0, 4, 4.0, 5, 5.0]
[1, 2, 3, 4, 5]

Rounding back to your question, I suppose HashSet() is a reasonably safe bet, since it doesn't do anything fanciful other than the usual hashCode()/equals() checks. And as you correctly inferred, since the method return type is just a Set, callers of union() should not have any other expectations - the Set interface simply says "this is how I am intended to be used.".

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.