Optional for an input parameter of a method

Question

I have created a generic interface which is implemented by certain classes. It looks like:

public interface KingdomElementService<T> {

  public T findById(Long id);

  public T save(Optional<T> object);

  public void update(T t);

}

My IntelliJ yells at save, writing "Optional is used as a parameter.". I understand the logic that it's weird to take in something that may exist and may not. Why I considered to do this is to rewrite some

 if(something){
   do();
 }
 else{
 throw new Expression();
 }

to

something.orElseThrow(Expression::new);

Now I am wondering if my solution of using an Optional is an overkill here or not. What do you think? Should I consider changing it back?

The Optional class doesn't make sense as a parameter How can you save something that doesn't exist? — Alan Kavanagh, Feb 13 '19 at 09:52
https://stackoverflow.com/questions/31922866/why-should-java-8s-optional-not-be-used-in-arguments — BretC, Feb 13 '19 at 09:53
I know what you mean. As I have written it, my reason of using optional was nothing like that. My object IRL is not nullable. I merely used optional to be able to reduce the 4-line if statement to a 1-line optional statement which does the same. — lyancsie, Feb 13 '19 at 09:54
It's not clear what the `Optional object` is meant to be for in the context of saving. It might be easier to give an answer with more information about that. — Andy Turner, Feb 13 '19 at 10:17
I am not pretty sure what code snippets should I copy to make it cleaner, but the implementations of this interface save values to their own repositories. Please write me if I haven't answered your question. — lyancsie, Feb 13 '19 at 11:33

score 3 · Answer 1 · answered Feb 13 '19 at 09:58

Nothing specific against Optional but by using this as a parameter, you are forcing all the callers to pass an Optional to this method. I would rather use @Nullable for this, e.g.:

public T save(@Nullable T object);

This would mean you will have to write that if..else block (or ternary operator) but in my opinion, the complexity of doing that will be far less than forcing your clients to pass an Optional.

Also, you can write Optional.of(object); as a first line in your method implementation and use it for the rest of the method code if you want to use the same control structure.

Sid · Accepted Answer · 2019-02-13T10:37:23.173

3

The Java language architects share the sentiment that Optional should be used only as return type. This is to signify that return type may be null. So care should be taken.

Optional as input parameter will require you to always check if the parameter is present, adding some checks. If you want to add any validations, you can put it without Optional as well . There is no extra value in marking input parameters as Optional.

Also the calling method that passes the parameter to another method accepting Optional will have to wrap the Object and send. This is an overhead.

I have not considered Edge cases.

EDIT: Changed from 'Java language is quite clear' to something more truthy.

edited Feb 13 '19 at 10:37

answered Feb 13 '19 at 09:59

Sid

4,893
14
55
110

3

"quite clear in documentation" please provide a link to the specific documentation which says this. The *language* doesn't even mention `Optional`, because it's defined in the API; the API documentation for [`Optional`](https://docs.oracle.com/javase/8/docs/api/java/util/Optional.html) doesn't say anything about this. – Andy Turner Feb 13 '19 at 10:07
1

@AndyTurner It's not documentation, but [Brian Goetz has expressed a similar sentiment here before](https://stackoverflow.com/a/26328555/1898563). – Michael Feb 13 '19 at 10:22
@Michael that answer basically says "don't use it that way because we didn't intend it". I use an old gym card to scrape ice off my car windscreen: I'm pretty sure that it was never intended to be used that way, but it is effective. I hope the analogy is apparent: that answer doesn't really give a concrete reason *not* to use `Optional` in this way. – Andy Turner Feb 13 '19 at 10:27
@AndyTurner I guess you need to read between the lines a bit. Using Optional as a parameter does not reduce the risk of any errors, it merely adds verbosity. – Michael Feb 13 '19 at 10:33
@AndyTurner I apologize if that did not come out right. The official language specification does not mention it. But the Java language architects have shared this sentiment. – Sid Feb 13 '19 at 10:34
@AndyTurner I edited it already :) . thanks for your feedback. – Sid Feb 13 '19 at 10:37

score 1 · Answer 3 · answered Feb 13 '19 at 09:58

When you have a method like save(Optional<T> object);

I still can make a call save(null); that's why optional here does not make sense.

You can implement a simple null check in your method

void save(T object){
        if(object == null){
            // do smth
        }
    }

Or use Optional.ofNullable() inside the method

void save(T object){
    Optional.ofNullable(object)
        .map(..)
    .orElseGet(IllegalAccessError::new);
}

score 1 · Answer 4 · answered Feb 13 '19 at 20:16

It seems like if you have an instance of KingdomElementService<T>, you want callers of save to have the choice of passing an instance of T or not. Thus, given some variables:

KingdomElementService<T> kes = ... ;
T someT = ... ;

Callers can do this:

kes.save(Optional.of(someT)); // 1
kes.save(Optional.empty());   // 2

This makes things quite inconvenient for callers. Instead, you should modify KingdomElementService to provide overloads of the save method: one taking an arg and one taking no args:

interface KingdomElementService<T> {
    T save(T t);
    T save();
}

If you do this, the callers don't have to bother wrapping everything inside an Optional:

kes.save(someT); // 1
kes.save();      // 2

Essentially, Optional doesn't help APIs that want to take optional parameters.

Optional for an input parameter of a method

4 Answers4