2

I used to call for Collections.emptyList(), emptySet and emptyMap to return a reference to an immutable empty collection instead of null from my functions. That's because I see no reasons to allocate a new empty collection instance:

private static Collection<Cheese> getCheesesOld() {
    if (cheesesInStock.isEmpty()) {
        return Collections.emptyList();
    }
    return new ArrayList<>(cheesesInStock);
}

Today I've read the following in Item 54 of Effective Java 3rd edition:

... you can avoid the allocations by returning the same immutable empty collection repeatedly, as immutable objects may be shared freely... But remember, this is an optimization, and it's seldom called for. If you think you need it, measure the performance before and after, to ensure that it's actually helping.

For me it sounds like I should avoid returning empty immutable collections until I can prove it will increase the performance. The question is: why? Are there any other drawbacks of returning immutable empty collections besides you can't mutate it and there are more code to type?

Should I change my default behavior and defensively return a copy (Item 50) of an internal list even if it is empty instead of returning already allocated Collections.emptyList()?

Kirill
  • 6,762
  • 4
  • 51
  • 81
  • 1
    The other drawback is additional complexity (more code to write, read and understand), and additional documentation needed: the caller might think that he/she can freely modify the returned list, until he/she hits the exceptional empty case, and he/she can't anymore. – JB Nizet Jun 16 '19 at 10:05

2 Answers2

1

Collections.emptyList() do not allocate an internal array, know their size instantly, might have better Iterators/Spliterators written for them (matters when you call .stream()); on the other hand that if branch might be more expensive than the gain you would have from Collections.emptyList().

The usual advice here is - use whatever works and reads easier for you. One place where this optimization could be present, but it's not, is in the Collectors.toList(): it always returns an empty ArrayList, instead of potentially returning Collections.emptyList() in java-8. I assume that is done specifically because the checks for such a rarely used scenario would introduce penalties in the frequent used scenarios; thus not done.

You also have to take that advice from the book with a grain of salt - that man writes libraries used by millions (very good advices there), but are you doing the same?

Eugene
  • 117,005
  • 15
  • 201
  • 306
  • There’s a small difference between `Collectors.toList()` and the scenario of this question. With the collector, an new `ArrayList` is allocated and when no eleemnt is ever being added, it has no array on its own. In contrast, the `ArrayList(Collection)` constructor will always invoke `toArray()` on the source collection, potentially creating an unshared empty array in the case of an empty collection, which only subsequently gets replaced by a shared empty array. – Holger Jun 17 '19 at 11:47
1

We can see it the other way round. Apparently, the caller of this method is not supposed to assume a mutable list, as otherwise returning an immutable list would not work.

Then, the fact that in some circumstances an actually mutable ArrayList is returned, which does not enforce the immutability, is an optimization.

It’s not so big, considering

private static Collection<Cheese> getCheesesOld() {
    if(cheesesInStock.isEmpty()) {
        return Collections.emptyList();
    }
    return Collections.unmodifiableList(new ArrayList<>(cheesesInStock));
}

which is only adding a small overhead, performance-wise or from a code development/maintenance perspective. The three lines of code specific to handling an empty lists weigh more.

But you may still feel uncomfortable with now returning three new wrapped objects in case of an empty list, if you remove the special handling, when returning a shared object would be enough. That feeling may persist, even when you realize that the scenario almost never occurs, i.e. when the list is almost never empty when the method is invoked.

Thankfully, there’s a simple solution with recent Java versions. You may use

private static Collection<Cheese> getCheesesOld() {
    return List.copyOf(cheesesInStock);
}

with Java 10 or newer. The returned list is immutable, but not a list wrapped in another guarding object and it will have optimized versions, not only for empty lists but also for lists of small sizes for which the JRE developers considered it beneficial.

It’s also smart enough not to do an actual copy operation if the source is already an immutable list, which implies that using the same method for creating defensive copies of incoming lists will pay off when they are already copies made by such getter methods.

You only have to cope with the new null handling, i.e. the radical rejection of null elements.

Holger
  • 285,553
  • 42
  • 434
  • 765