ImmutableList.of() --> Collections.emptyList() about presto

[ScrapCodes]

May be I can try this!

[ScrapCodes]

find . -type f -name '*java' -exec grep -nHI -e "ImmutableList.of()" \{\} + | wc -l
2078

[elharo]

I don't think that will work since you didn't include the parentheses. Only no-args ImmutableList.of() can be replaced by Collections.emptyList().

[ScrapCodes]

@elharo understood that and corrected myself.

[tdcmeehan]

@elharo what's the motivation for this change?

[elharo]

tldr; JDK > 3rd party library

https://stackoverflow.com/questions/43291202/collections-emptylist-vs-guavas-immutablelist-of

[tdcmeehan]

@elharo from the SO post, I see one answer that lists two reasons:

1. Why introduce a third party dependency when you can use whatever is already built-in to the JDK?
2. The JDK factory uses a dedicated implementation for the empty list, vs. Guava uses an immutable list with no elements.

For point 1, I don't understand what the benefit is, unless we have some sort of overarching goal to remove our Guava dependency. We already have this dependency because we prefer immutability, so I don't understand why we would want to differentiate empty vs. non-empty immutable collections.

For point 2, I simply don't understand what sort of practical benefit this has. Can you attempt to explain it?

[elharo]

I don't understand or care about #2 either. I do think #1 is important though for a number of reasons:

1. It's more familiar to new developers. No third party library is going to be as well known as what's already in the JDK. The code is simply clearer to more people when it uses the standard libraries.
2. The JDK has been far more stable over time than any third party library, Guava included. I've seen Guava go through cycles of well supported and not so well supported over the years. I've also watched compatibility promises change over the years (generally for the better). And of course it's far from out of the question that Google loses interest in Guava, lays off or reassigns the team, and ignores it for multiple years.
3. Standard libraries are much more likely to be optimized in VMs than third party libraries.
4. In this specific case, "emptyList" is a better name that makes the code clearer than "of". It more obviously expresses the intent

Bottom line: ImmutableList.of() is a relic of quite old JDKs. Now that Presto is on JDK 8, it's long past the point where this was useful.

[tdcmeehan]

I think arguments 1 and 2 would make more sense to me if there were suitable immutable alternatives for collections in the JDK. Such alternatives don't exist. Which leaves us with one way of doing things for empty lists, and a different way of doing things for non-empty lists. We still have the problem you mention of relying on Guava, because our non-empty lists will still use it. And in terms of familiarity, to me this just seems really hard to quantify in a codebase which already has so many quirks not found in many other Java projects. In short, I just view this as having a cost (two ways of creating immutable lists instead of one), and I'm not really convinced of the benefits.

For point 3, I would want to see something that proves we get some sort of non-negligible improvement, otherwise it just risks being a micro-optimization.

[tdcmeehan]

I'm going to remove the good first issue tag to allow time for alignment around this, we can add it back later.

[elharo]

It doesn't help that the class hierarchy for lists has been borked since Java 1.1. The Liskov Substitution Principle requires that MutableList be the subtype and List not have any methods that ever throw UnsupportedOperationException, but I'm afraid that ship sailed long ago. :-(