CSnerd
CSnerd

Reputation: 2239

Use overloading judiciously

The constructors for TreeSet include, besides the standard ones, one which allows you to supply a Comparator and one which allows you to create one from another SortedSet:

TreeSet(Comparator<? super E> c)
// construct an empty set which will be sorted using the
// specified comparator 
TreeSet(SortedSet<E> s)
// construct a new set containing the elements of the 
// supplied set, sorted according to the same ordering

The second of these is rather too close in its declaration to the standard "conversion constructor” :

TreeSet(Collection<? extends E> c)

As Joshua Bloch explains in Effective Java (item “Use overloading judiciously” in the chapter on Methods), calling one of two constructor or method overloads which take parameters of related type can give confusing results. This is because, in Java, calls to overloaded constructors and methods are resolved at compile time on the basis of the static type of the argument, so applying a cast to an argument can make a big difference to the result of the call, as the following code shows:

// construct and populate a NavigableSet whose iterator returns its
// elements in the reverse of natural order:
NavigableSet<String> base = new TreeSet<String>(Collections.reverseOrder());
Collections.addAll(base, "b", "a", "c");

// call the two different constructors for TreeSet, supplying the
// set just constructed, but with different static types: 
NavigableSet<String> sortedSet1 = new TreeSet<String>((Set<String>)base);
NavigableSet<String> sortedSet2 = new TreeSet<String>(base);
// and the two sets have different iteration orders: 
List<String> forward = new ArrayList<String>(); 
forward.addAll(sortedSet1);
List<String> backward = new ArrayList<String>(); 
backward.addAll(sortedSet2);
assert !forward.equals(backward); 
Collections.reverse(forward); 
assert forward.equals(backward);

This problem afflicts the constructors for all the sorted collections in the Framework (TreeSet, TreeMap, ConcurrentSkipListSet, and ConcurrentSkipListMap). To avoid it in your own class designs, choose parameter types for different overloads so that an argument of a type appropriate to one overload cannot be cast to the type appropriate to a different one. If that is not possible, the two overloads should be designed to behave identically with the same argument, regardless of its static type. For example, a PriorityQueue constructed from a collection uses the ordering of the original, whether the static type with which the constructor is supplied is one of the Comparator-containing types PriorityQueue or SortedSet, or just a plain Collection. To achieve this, the conversion constructor uses the Comparator of the supplied collection, only falling back on natural ordering if it does not have one.

Currently, I am reading the book called Java Generics and Collections, and above is something I don't understand [page185~186].

First, I don't quite understand why it uses this example and what things it want to illustrate.

Second, I don't quite understand the concept of "conversion constructor". Is it because of the existence of conversion constructor, that we should use overloading judiciously?

Upvotes: 8

Views: 581

Answers (3)

yshavit
yshavit

Reputation: 43401

The problem is that the two constructors have slightly different behavior, and thus violates the so-called "principle of least astonishment".

TreeSet(SortedSet<E>) constructs a new set "using the same ordering as the specified sorted set," whereas TreeSet(Collection<? extends E>) uses the "natural ordering of its elements." This means that two TreeSets constructed with the same underlying instance can act a bit different, depending on the static type of the reference they were constructed with.

SortedSet<Integer> original = getReverseSet(); // { 5, 4, 3, 2, 1}
Collection<Integer> alsoOriginal = original; // same instance exactly

TreeSet<Integer> a = new TreeSet<>(original);
TreeSet<Integer> b = new TreeSet<>(alsoOriginal);

It looks at first blush that a and b should be identical -- after all, they were constructed using the same exact instance! But the first uses the TreeSet(SortedSet) constructor (and thus preserves the reverse ordering), while the second uses the TreeSet(Collection) constructor (and thus uses the elements' natural ordering, which is different than the reverse ordering). In addition, a.comparator() will return the reverse comparator, whereas b.comparator() will return null.

This isn't wrong per se, but it can be surprising and confusing to users of your library!

Upvotes: 4

kervin
kervin

Reputation: 11858

First, I don't quite understand why it uses this example and what things it want to illustrate.

He is illustrating the fact that inheritance is allowing the compiler to make decisions he believes the developer should make explicitly.

I've read the book as well. The first half is very helpful in understanding Java's confusing Generics.

The compiler is picking

TreeSet(Collection<? extends E> c)

or...

TreeSet(SortedSet<E> s)

Just by a simple cast. But we are passing in the same base variable. He is saying this is too close for comfort. You should not create Conversion Constructors which parameters are in the same type hierarchy. I.e. SortedSet is a Collection.

Second, I don't quite understand the concept of "conversion constructor".

Effective Java, Page 61 defines a "Conversation Constructor" as a "Copy Constructor". He mentions...

Interface-mapped Copy Constructors and... more properly known as Conversion Constructors...

Also from https://docs.oracle.com/javase/tutorial/collections/interfaces/collection.html

This constructor, known as a conversion constructor, initializes the new collection to contain all of the elements in the specified collection, whatever the given collection's subinterface or implementation type. In other words, it allows you to convert the collection's type.

Upvotes: 0

Aivean
Aivean

Reputation: 10882

Provided code snippet compiles just fine. Make sure that you have imported TreeSet from java.util package (single import import java.util.*; should be enough for this code to compile).

Regarding your questions:

  1. This example illustrates that sometimes it's hard to tell what method will be called if you have two overloaded methods with arguments that can be in single class hierarchy. In given example, base is SortedSet and Collection in the same time. When you calling constructor of TreeSet and passing base as parameter, which constructor is invoked depends on compile-time type of base. So, when you are passing it "as is", it's type will be TreeSet, and the constructor with most concrete type will be chosen (one that takes SortedSet). When you are explicitly casting base to Set, it's compile-time type changes and hence changes the invoked constructor will be one that takes Collection.

  2. "Conversion constructor" in this case is just the constructor that takes elements of one collection and creates another collection. The "use overloading judiciously" rule can be applied to any method, it doesn't have to be constructor.

For example, take a look at remove(int|Integer) method of List<Integer> and try to figure out which one will be invoked in which case.

Upvotes: 0

Related Questions