10

Is this a valid (intended) usage of Optional type in Java 8?

String process(String s) {
    return Optional.ofNullable(s).orElseGet(this::getDefault);
}
Łukasz Rzeszotarski
  • 5,312
  • 6
  • 33
  • 61
  • `.map(f -> true).orElse(false);` is an obscure way to say `.isPresent()`? – Tagir Valeev Dec 24 '15 at 07:28
  • Didn't mean that. A way to say if present return some object else return another object. Boolean maybe was not best choice in my example – Łukasz Rzeszotarski Dec 24 '15 at 08:22
  • `Optional` is meant to prevent `NullPointerException`s and `getFoo` produces one. And the question is about the valid usage of `Optional`. That's what I call irony. – a better oliver Dec 24 '15 at 08:33
  • @zeroflagL getFoo doesn't cause null pointer exception and someone wrote it did. This is what I call irony. – Łukasz Rzeszotarski Dec 24 '15 at 08:45
  • `id` can be `null` and you have `id == 1`. – a better oliver Dec 24 '15 at 08:47
  • yes but this is not the point here. Should I use Optional ;) I guess not :) – Łukasz Rzeszotarski Dec 24 '15 at 08:51
  • You create an `Optional` anyway. You could as well write `return Optional.ofNullable(id).filter(i -> id == 1).map(i -> new Foo(id, "Bar", "US"))`. – a better oliver Dec 24 '15 at 09:11
  • @ŁukaszRzeszotarski "Question: why -1" - Try spending more time actually asking a question with body. Question which are 95% code are easily downvoted for any number of reasons and assumptions. Example: add what you've reasoned yourself, so people know where you're going with this. – Gimby Dec 24 '15 at 09:23
  • 1
    @ŁukaszRzeszotarski has some point here. Have a look at the `javax.money` API - they are using the same construct: https://github.com/JavaMoney/jsr354-ri/blob/master/src/main/java/org/javamoney/moneta/ToStringMonetaryAmountFormat.java#L47 – Petar Tahchiev Dec 24 '15 at 09:25
  • @PetarTahchiev yes after the discussion I came to the conclusion that the whole concept of Optional in jdk is really sad. While designers thoughts, what he or they meant is not enough to use the new api correctly. If it is only for some reasons good it should be either limited in the public Api or at least well documented. I believe many misuse the concept because of these reasons. – Łukasz Rzeszotarski Dec 24 '15 at 10:00
  • @Gimby As I see for all is everyone who commented or answered is clear what I was asking about – Łukasz Rzeszotarski Dec 24 '15 at 11:13
  • @PetarTahchiev, `String.valueOf(amount)` is a much shorter alternative to do the same which exists since JDK 1.0. – Tagir Valeev Dec 24 '15 at 12:08

4 Answers4

24

I'll take another swing at this.

Is this a valid usage? Yes, in the narrow sense that it compiles and produces the results that you're expecting.

Is this intended usage? No. Now, sometimes things find usefulness beyond what they were originally for, and if this works out, great. But for Optional, we have found that usually things don't work out very well.

Brian Goetz and I discussed some of the issues with Optional in our JavaOne 2015 talk, API Design With Java 8 Lambdas and Streams:

The primary use of Optional is as follows: (slide 36)

Optional is intended to provide a limited mechanism for library method return types where there is a clear need to represent "no result," and where using null for that is overwhelmingly likely to cause errors.

The ability to chain methods from an Optional is undoubtedly very cool, and in some cases it reduces the clutter from conditional logic. But quite often this doesn't work out. A typical code smell is, instead of the code using method chaining to handle an Optional returned from some method, it creates an Optional from something that's nullable, in order to chain methods and avoid conditionals. Here's an example of that in action (also from our presentation, slide 42):

// BAD
String process(String s) {
    return Optional.ofNullable(s).orElseGet(this::getDefault);
}

// GOOD
String process(String s) {
    return (s != null) ? s : getDefault();
}

The method that uses Optional is longer, and most people find it more obscure than the conventional code. Not only that, it creates extra garbage for no good reason.

Bottom line: just because you can do something doesn't mean that you should do it.

Community
  • 1
  • 1
Stuart Marks
  • 120,620
  • 35
  • 192
  • 252
  • The first constructive answer. Thank you! My question was quite massively downvoted, though I think it quite reasonable. Should I rephrase it? – Łukasz Rzeszotarski Dec 24 '15 at 22:58
  • @ŁukaszRzeszotarski Thanks. Hard to say what to do about downvotes. The downvotes (and close-votes) probably come from people who perceive that the question is opinion-based or that you're looking for a discussion/argument. Avoid that, I guess, but I'm not sure what to do about this particular question. – Stuart Marks Dec 25 '15 at 04:10
  • @StuartMarks **Optional is very valuable on APIs (public methods on Interfaces or Classes), when I want to represent the possibility of absence of a returned value**. In `public interface OrderService { Optional findOrderWithId(long id);}`, _the return type says that `Order` may be absent_. In the client code it reduces the chance of forgetting to handle `null` order and thus a lesser chance of a potential `NullPointerException`. More details: [Using Optional to Specify Absence](http://praveer09.github.io/technology/2015/09/19/using-optional-to-specify-presence-or-absence-of-a-value/) – Praveer Gupta Dec 27 '15 at 04:13
6

Since this is more or less an opinion-based question, I'll throw mine in. If you're trying to say

if (id == 1) {
    Foo f = new Foo(id, "Bar", "US");
    return "Bar".equals(f.getName()) && "US".equals(f.getCountryCode());
} else {
    return false;
}

then just say that. Making things "functional" doesn't automatically make things clearer or better. By introducing a needless Optional, a couple lambdas, and some Optional methods that I had to look up, you've made the code more convoluted and difficult to understand. I don't think the designers of Java "intended" for people to use Optional to help make code more obscure.

EDIT: After reading some responses, I think it's worth adding some comments. This is not a functional programming idiom I'm familiar with, which would make it harder to understand. The idioms I am familiar with mostly involve Java streams, or (in other languages) functional idioms applied to multiple values in arrays or lists or other collections of multiple values. In those cases, once you get past the unfamiliarity, the functional syntax can be seen as an improvement because it allows some details to be hidden (loop indexes, iterators, running pointers, accumulator variables). So overall, it can simplify things. This example, by itself, doesn't do any such simplification.

However, some of the Optional features are useful in stream contexts. Suppose we had a parseInt() method that returns an Optional<Integer>, which is empty if the input string is invalid. (Java 8 really should have provided this.) This would make it easy to take an array of strings and produce an array of integers in which the strings that don't parse are simply eliminated from the result--use parseInt in a stream map(), and use a stream filter to filter out the empty Optionals. (I've seen multiple StackOverflow questions asking how to do this.) If you want to keep only the positive values, you could use an Optional.filter() to change the nonpositives to Optional.empty() before using the stream filter (although in this case, you could add another stream filter afterwards, but in a more complex case the Optional filter could be more useful). That's what I see as the main benefit of Optional from a functional standpoint. It allows you to work with a collection of values all at once, by giving you a way to represent "non-values" and write a function that will still work with them. So I guess the main use of Optional, besides a replacement for null, would be to represent empty spaces in a sequence of values while you're applying functions to the entire sequence as a whole.

ajb
  • 30,640
  • 3
  • 52
  • 82
  • Hmmm in principle I don't agree. You think it is easier to read because you are accustomed to some syntax. For me readability for both is the same. But agree with yhsavit that when designers made it for another purposes it shouldn't be used like that. – Łukasz Rzeszotarski Dec 24 '15 at 08:27
  • One punkt more is ... as I said I agree with yshavit that when intention is to use it only for public methods that possible may return null it should be use like that. But another issue is a filter syntax, which you don't like. Ir then why it is in public Optional api? For me conclusion is that Optional type has a very limited usage in Java 8, and actually is not as great feature as many think. – Łukasz Rzeszotarski Dec 24 '15 at 09:00
4

Asking whether it's "valid" is rather opinion-based, but as to whether it's the intended use case: no, it's not.

Brian Goetz, Oracle's language architect for Java, has stated that the use case for Optional is for when you need a "no value" marker, and when using null for this is likely to cause errors. Specifically, if a reasonable user of your method is not likely to consider the possibility that its result is null, then you should use Optional. It was explicitly not intended to be a general "Maybe"-type object, as you're using it here.

In your case, the method that returns the Optional is private. That means it can only be used by the implementers of the class, and you can assume that they have good knowledge of the class' methods — including which of them may return null. Since there's no reasonable risk of confusion, Brian Goetz would (probably) say that he would not consider this a valid use case.

Community
  • 1
  • 1
yshavit
  • 41,077
  • 7
  • 83
  • 120
  • Would be your answer 'Yes' if it was public method? – Łukasz Rzeszotarski Dec 24 '15 at 05:58
  • @ŁukaszRzeszotarski The answer would then be "it depends." Specifically, on whether you think that just returning null is "overwhelmingly likely to cause errors" (quote taken from Goetz's answer that I linked to in my answer). That would be a judgement call we can't make from this snippet of code. – yshavit Dec 24 '15 at 06:00
  • I consider this statement a bit confusing 'Brian Goetz, Oracle's language architect for Java, has stated that the use case for Optional is for when you need a "no value" marker, and when using null for this is likely to cause errors. ' Would be great if the api would be so designed/ documented that its usage would be clear without interprating stackoverflow statements of Mr. Brian Goetz. But as I understood you your opinion is that it makes sense only for public methods, which are supposed to return null values?! – Łukasz Rzeszotarski Dec 24 '15 at 06:13
  • Yup, that's about the gist of it. As for what might have been great or not, that's opinion-based and off topic for SO. I tried to keep to the objective facts, which seem to indicate that by the designers' original intention, private methods should never return Optional (and it's a judgement call as to which public methods should return it). – yshavit Dec 24 '15 at 06:26
  • Your last paragraph is based entirely on assumptions. According to you it's off topic. – a better oliver Dec 24 '15 at 08:39
  • @zeroflagL: There's pure opinion, and there's judgement call. To make the judgment call that the implementer of a class should know the methods of that class is not off topic. It's like the difference between "what's the best flavor of ice cream?" and "is eating two pints of ice cream every day healthy?" – yshavit Dec 24 '15 at 15:53
2

Its a little contrived, but 'valid' (as in 'syntactically') , but as @yshavit pointed to, it was intended for use in library development.


Previous answer was due to FP code being difficult to read. Below is commented(a little verbose, b/c that is the javadoc comments) but still. Much easier to read IMHO. (2nd is no-comments, and at least alignment to help readability)

private boolean isFooValid(final Integer id) {
    return getFoo(id)
        // filter if 'f' matches the predicate, return Optional w/f if true, empty Optional if false
        .filter(f -> "Bar".equals(f.getName()) && "US".equals(f.getCountryCode())) 
        // If a value is present, apply the provided mapping function to it, 
        // If non-null, return an Optional describing the result.  
        .map(f -> true)
        // Return the value if present, otherwise return other.
        .orElse(false);
 }

Or at least line it up so its more apparent what is going on and easier to read.

private boolean isFooValid(final Integer id) {
    return getFoo(id)
        .filter(f -> "Bar".equals(f.getName()) && "US".equals(f.getCountryCode())) 
        .map(f -> true)
        .orElse(false);
 }
mawalker
  • 2,097
  • 2
  • 20
  • 31
  • Wrong, I think, because if `getFoo(int)` returns an empty `Optional`, `filter` will not call `getName()`, which is part of a lambda. – ajb Dec 24 '15 at 07:40
  • Updated after you pointed out 'filter'. Didn't even _see_ the word when i looked the first time :/ – mawalker Dec 24 '15 at 07:59