HashSet calls overridable method in constructor

It is bad practice to call overridable methods in a constructor (see this link). This will always call the method defined in the class and not a derived class' method. In Java's HashSet there is a constructor that accepts a Collection. This method delegates to the addAll method. My question is why won't this break any derived classes of HashSet.

Constructs a new set containing the elements in the specified collection. The HashMap is created with default load factor (0.75) and an initial capacity sufficient to contain the elements in the specified collection. Parameters: c the collection whose elements are to be placed into this set Throws: java.lang.NullPointerException if the specified collection is null

public HashSet(Collection<? extends E> c) {
    map = new HashMap<E,Object>(Math.max((int) (c.size()/.75f) + 1, 16));
    addAll(c);
}
Jon Skeet
people
quotationmark

The answer is that it does break subclasses, potentially. It's easy to show that:

BrokenSet.java:

import java.util.*;

public class BrokenSet<E> extends HashSet<E> {
    private final List<E> list = new ArrayList<E>();

    public BrokenSet(Collection<? extends E> c) {
        super(c);
    }

    @Override public boolean add(E item) {
        if (super.add(item)) {
            list.add(item);
            return true;
        }
        return false;
    }
}

Main.java:

import java.util.*;

public class Main{

    public static void main(String[] args) {
        Collection<String> strings = Arrays.asList("x", "y");
        Set<String> set = new BrokenSet<>(strings);
    }

}

Run Main, and you'll get:

Exception in thread "main" java.lang.NullPointerException
        at BrokenSet.add(BrokenSet.java:12)
        at java.util.AbstractCollection.addAll(Unknown Source)
        at java.util.HashSet.<init>(Unknown Source)
        at BrokenSet.<init>(BrokenSet.java:7)
        at Main.main(Main.java:7)

... because while the superconstructor is running, it calls BrokenSet.add which expects list to be non-null.

Subclasses would need to be written carefully to avoid this being an issue. (You might want to look at what LinkedHashSet does, for example.)

people

See more on this question at Stackoverflow