Null-Check & isPresent - Different name but same issue?

Question

I have the following code in Java:

public class Browser {

  public URL back() {
    try {
      //simulate: fetch last URL from Stack
      return Math.random() < 0.5 ? new URL("http://google.de") : null;
    } catch(MalformedURLException e) {
      return null;
    }
  }

  public void retrieveSite(URL url) {
    System.out.println(url);
    //...
  }

  public static void main(String[] args) {
    System.out.println("Normal back");
    Browser browser = new Browser();
    URL back = browser.back();
    if (back != null) browser.retrieveSite(back);
  }
}

I want to learn more about Optional and re-write this code so that return null and if (back!=null) is not required anymore.

So this is what i got:

public class Browser {

  Optional<URL> url = Optional.empty();

  public Optional<URL> back() {

    try {
      //simulate: fetch last URL from Stack
      if(Math.random()<0.5) {
        url = Optional.of(new URL("http://google.de"));
      } 
      return url;
    } catch(MalformedURLException e) {
      return url;
    }
  }

  public void retrieveSite(Optional<URL> url) {
    System.out.println(url);
    //...
  }

  public static void main(String[] args) {
    System.out.println("Normal back");
    Browser browser = new Browser();
    Optional<URL> back = browser.back();
    if(back.isPresent()) {
      browser.retrieveSite(back);
    }   
  }
}

Now in order to avoid passing an empty Optional to retrieveSite, i have to check for a present value. But what exactly am I gaining from checking for isPresent instead of just !=null? Should i return a default value instead, so i can get rid of isPresent?

Also i had to change the Parameter for retrieveSite() to take an Optional which is considered as bad practice?

Thanks in advance.

The point about `Optional` is not that it obviates the need to do checks, necessarily: it is that you can't treat an `Optional` as a `URL`, so you *have to* handle the presence (or absence) explicitly. The same is not true of a null `URL`: you can pass `null` where a non-null `URL` is expected, and you don't find out until you try to dereference it. — Andy Turner, May 21 '19 at 16:53

score 1 · Answer 1 · answered May 21 '19 at 16:55

A slightly different variant for your second approach could be implemented in a much cleaner way as:

static class Browser {

    // good practice to return Optional instead of a null 
    Optional<URL> back() {
        try {
            //simulate: fetch last URL from Stack
            return Math.random() < 0.5 ? Optional.of(new URL("http://google.de")) : Optional.empty();
        } catch (MalformedURLException e) {
            return Optional.empty();
        }
    }

    // avoid using Optional as a parameter to a method
    static void retrieveSite(URL url) {
        System.out.println(url);
    }

    public static void main(String[] args) {
        System.out.println("Normal back");
        Browser browser = new Browser();
        // perform a void operation if the URL is present (consuming it)
        browser.back().ifPresent(Browser::retrieveSite);
    }
}

you mentioned that i should avoid using Optional as a parameter to ``retrieveSite`` and i read many times that its considered bad practice. But why exactly is that the case? — Clayy91, May 21 '19 at 21:30
@Clayy91 There are already few explanations in [this post](https://stackoverflow.com/questions/31922866/why-should-java-8s-optional-not-be-used-in-arguments) for the same. — Naman, May 22 '19 at 01:25

score 0 · Accepted Answer · answered May 21 '19 at 17:15

With Optional you have to unwrap/test the Optional to get it and you also get early emptiness exception if Optional is not correctly used (fail fast principle).
For example :

  public static void main(String[] args) {
    System.out.println("Normal back");
    Browser browser = new Browser(); 
   // unwraping the optional or testing it is mandatory to get the object inside in
    browser.back().ifPresent(browser::retrieveSite); 
    // for example it will not compile
    browser.retrieveSite(browser.back()); 
    // of course you could cheat by invoking Optional.get() but that is a bad practice and the absence of object will be detected as soon as the invocation 
    browser.retrieveSite(browser.back().get()); 
  }

  public void retrieveSite(URL url) {
    //...
  }

Without Optional, a NPE is possible if the client forget to check explicitly the no nullity (url != null). This check is really less compelling for the developper that a method invocation that is mandatory to get/map the wrapped object. Besides, you can discover the null reference later in a very bottom layer if the url parameter is passed through the layers, which makes issues potentially more complicated to understand and solve :

  public static void main(String[] args) {
    System.out.println("Normal back");
    Browser browser = new Browser(); 
    // No unwrapping is necessary to get the url. 
    // So the robustness of the code depends on the developer habits
    browser.retrieveSite(browser.back());     
  }

  public void retrieveSite(URL url) {        
    //...
  }

It might be worth noting (explicitly) that by looking at a method return type of `URL`, you don’t see that it could be `null` whereas a return type of `Optional` immediately shouts that it could be absent. Further, `isPresent()` has the same meaning as “is not `null`” of the previous version, because `null` had the “is absent” meaning, in this specific example. But `null` does not always have that meaning. Sometimes, `null` means “I forgot to initialize that field”, in which case I’d never switch to `Optional`. — Holger, May 22 '19 at 11:04
@Holger Agreed for the first part. For "Sometimes, null means “I forgot to initialize that field”, in which case I’d never switch to Optional." I don't. Optional should not be used for fields. So no real debate about it. Now checking not null for a field may of course makes sense but this is different of the actual use case : method invocation that return something. — davidxxx, May 22 '19 at 18:59
Of course, you should not use optional for fields at all. But when you create a method returning the value of a field, you may decide to make the method’s return type an `Optional` if the field can be `null` or not an optional, if it is supposed to be never `null`. — Holger, May 23 '19 at 07:16

Null-Check & isPresent - Different name but same issue?

2 Answers2