Is Optional bad practice?

Question

I keep seeing some users saying that using a collection with an optional is not recommended / is bad practice. I couldn't find much information on why it would be bad practice, so here I am.

In case it matters, in my particular case I have a JpaRepository with 2 methods. One Optional<List> getValues() and a second List getDefaultValues(), as such:

public interface someRepository extends JpaRepository<x, y> {

    Optional<List> getValues();

    List getDefaultValues();
}

The idea is that the values might not be present so I want to force the method caller to do .orElse(getDefaultValues()) so there is no chance getting returned an empty List. I also figured it would be tidier than just doing something like this:

List list = getValues();

if (list.isEmpty()) {
   list = getDefaultValues();
}

Any feedback is appreciated.

Edit: I realise my example is not the most well suited to the question, as pointed out in the answers below. However, I would still like to understand why Optional<List> is considered bad practice by some.

This question does not really answer my question.

Does this answer your question? [Java Optional questions, am I doing this right?](https://stackoverflow.com/questions/27102481/java-optionalt-questions-am-i-doing-this-right) — crissal, Aug 19 '21 at 09:57
Why don't you return the default values if the list is empty or null? — AndiCover, Aug 19 '21 at 09:58
_If_ you're going to use `Optional`, you'll want to use `.orElseGet()` instead of `orElse()`. See https://stackoverflow.com/questions/33170109/difference-between-optional-orelse-and-optional-orelseget — Ivar, Aug 19 '21 at 09:58
some reading: https://blogs.oracle.com/javamagazine/12-recipes-for-using-the-optional-class-as-its-meant-to-be-used https://blogs.oracle.com/javamagazine/the-java-optional-class-11-more-recipes-for-preventing-null-pointer-exceptions — thinkgruen, Aug 19 '21 at 10:00
Why not just have a single `List getValues()` method and hide the implementation details behind that? Or does the caller need to be aware that default values have been returned? — Robby Cornelissen, Aug 19 '21 at 10:01
I wish you say some reasons of those who don't support Optional. I'm on your side. — AMK, Aug 19 '21 at 10:02
AFAIK `isPresent` only checks for `null`, with that said, it would always return true. [here](https://github.com/AdoptOpenJDK/openjdk-jdk11/blob/master/src/java.base/share/classes/java/util/Optional.java#L158-L160) — SaleemKhair, Aug 19 '21 at 10:29

Robby Cornelissen · Accepted Answer · 2021-08-19T10:32:52.820

3

I think the point is moot. There are two possible cases:

1. The caller needs to be aware that default values have been returned
In this case, the caller will not be able to use the orElse()/orElseGet() construct, and will have to check with isPresent(). This is no better than checking whether the list is empty.

2. The caller does not need to be aware that default values have been returned
In which case you might as well hide the implementation details behind a single List getValues() method that returns the default values in case no values were found.

As to the general applicability of using Optional<List>, I think the Brian Goetz quote from this answer says it best:

Our intention was to provide a limited mechanism for library method return types where there needed to be a clear way to represent "no result", and using null for such was overwhelmingly likely to cause errors.

When it comes to lists (and collections in general), there is already a clear way to represent "no result", and that is an empty collection.

edited Aug 19 '21 at 10:32

answered Aug 19 '21 at 10:09

Robby Cornelissen

91,784
22
134
156

Those are good points. I might have just opted for Optional purely because it was the "cheaper" solution in the moment, though I realise your case 2 is probably the best approach. – valegians Aug 19 '21 at 10:15
I'd still like some thoughts on the overarching question though. I can't quite wrap my head around the bad practice argument. – valegians Aug 19 '21 at 10:17
3

@valegians My assumption would be that is inherited from the bad practice of returning `null` vs an empty collection. (See [this answer](https://stackoverflow.com/a/1970001). It's about C#, but the same applies to Java.) [This answer](https://stackoverflow.com/a/26328555) from Brian Goetz (one of Java's language architects) might also be relevant. – Ivar Aug 19 '21 at 10:32
@Ivar RobbyCornelissen Thank you both for the input. It makes a lot more sense now. – valegians Aug 19 '21 at 10:37

Stephen C · Answer 2 · 2021-08-19T14:46:17.730

Lets first consider this example:

public class <X> Example {

    public List<X> getValues() {
        if (...) {
            return null;
        } else {
            return /* a non-empty list */
        }
    }

    public List<X> getValuesAgain() {
        if (...) {
            return Collections.emptyList();
        } else {
            return /* a non-empty list */
        }
    }
}

Which of those ways of returning no values is better?

I would argue that the better way is (nearly always) the way that getValuesAgain does it; i.e. by returning an empty list. Consider the following.

With the first version, I need to test for a null before using the result:

  List<String> l = example.getValues();
  if (l != null) {
      for (s String: l) {
          /* do something */
      }
  }

With the second version, I can use the result directly:

  List<String> l = example.getValuesAgain();
  for (s String: l) {
      /* do something */
  }

In short, returning null makes life more complicated for the caller compared with returning an empty list. In theory, you may have saved the creation of an empty list object ... except that the javadoc for emptyList() states that it doesn't need to return a distinct list object each time. (And it typically won't!)

Now consider what Optional<List<String>> means: either a List<String> or no value. But as we have already seen, returning an empty list is simpler for the caller that returning a null. And the same logic applies here too.

So if getValues returns an Optional<List<X>>, we would need to use it like this:

  Optional<List<String>> l = example.getValues();
  if (l.isPresent()) {
      for (s String: l.get()) {
          /* do something */
      }
  }

Compare with the original getValuesAgain:

  List<String> l = example.getValuesAgain();
  for (s String: l) {
      /* do something */
  }

In short, using Optional to return a list or null is not an improvement.

The only scenario where I think you should contemplate using Optional<List<?>> as a return type is if the API needs to make a semantic distinction between an empty list versus no list.

The same applies for other collection types, and also for arrays, strings and other examples.

Another way of thinking about this is that the designers' intention for Optional is to provide a way to indicate there is "no result"; see Brian Goetz's answer to this question. But we don't need to indicate there is "no result" here since we can return an empty collection.

Is Optional bad practice?

2 Answers2