85

I'm trying to make something like this:

 private String getStringIfObjectIsPresent(Optional<Object> object){
        object.ifPresent(() ->{
            String result = "result";
            //some logic with result and return it
            return result;
        }).orElseThrow(MyCustomException::new);
    }

This won't work, because ifPresent takes Consumer functional interface as parameter, which has void accept(T t). It cannot return any value. Is there any other way to do it ?

RichardK
  • 3,228
  • 5
  • 32
  • 52
  • Possible duplicate of [Proper usage of Optional.ifPresent()](http://stackoverflow.com/questions/24228279/proper-usage-of-optional-ifpresent) – Krishnanunni P V Jan 05 '17 at 13:05

5 Answers5

103

Actually what you are searching is: Optional.map. Your code would then look like:

object.map(o -> "result" /* or your function */)
      .orElseThrow(MyCustomException::new);

I would rather omit passing the Optional if you can. In the end you gain nothing using an Optional here. A slightly other variant:

public String getString(Object yourObject) {
  if (Objects.isNull(yourObject)) { // or use requireNonNull instead if NullPointerException suffices
     throw new MyCustomException();
  }
  String result = ...
  // your string mapping function
  return result;
}

If you already have the Optional-object due to another call, I would still recommend you to use the map-method, instead of isPresent, etc. for the single reason, that I find it more readable (clearly a subjective decision ;-)).

Roland
  • 22,259
  • 4
  • 57
  • 84
32

Two options here:

Replace ifPresent with map and use Function instead of Consumer

private String getStringIfObjectIsPresent(Optional<Object> object) {
    return object
            .map(obj -> {
                String result = "result";
                //some logic with result and return it
                return result;
            })
            .orElseThrow(MyCustomException::new);
}

Use isPresent:

private String getStringIfObjectIsPresent(Optional<Object> object) {
    if (object.isPresent()) {
        String result = "result";
        //some logic with result and return it
        return result;
    } else {
        throw new MyCustomException();
    }
}
Vlad Bochenin
  • 3,007
  • 1
  • 20
  • 33
  • 2
    Note that the second option is not the recommended way to use Optionals – Kartik Chugh Jun 19 '20 at 21:40
  • 2
    `map` is not intended nor designed for this usage. It always has a return type, so it's better to do `return someOptional.orElseThrow(() -> ...);` -- it is unclear if OP intended to return something or not. – Josh M. Mar 04 '21 at 01:17
  • This is better answer to map version. – Raichu Aug 01 '21 at 05:16
  • @josh could you explain why do you think `map` shouldn't be used, and instead we must return an `Optional` value. Because returning `Optional` effectively means that you need to add few more lines later to see if the intended item is present or not. – Prasannjeet Singh Aug 26 '22 at 11:31
  • You don't return `Optional`. If you do `someOptional.orElseThrow(() -> ...)` then the underlying value is returned _if there is one_, else the exception you specified in the lambda is thrown. This is better than using `.map` because that function is meant to transform one thing into another, it is not necessary unless you're massaging the optional value in some way. – Josh M. Aug 27 '22 at 02:00
18

Use the map-function instead. It transforms the value inside the optional.

Like this:

private String getStringIfObjectIsPresent(Optional<Object> object) {
    return object.map(() -> {
        String result = "result";
        //some logic with result and return it
        return result;
    }).orElseThrow(MyCustomException::new);
}
marstran
  • 26,413
  • 5
  • 61
  • 67
15

I'd prefer mapping after making sure the value is available

private String getStringIfObjectIsPresent(Optional<Object> object) {
   Object ob = object.orElseThrow(MyCustomException::new);
    // do your mapping with ob
   String result = your-map-function(ob);
  return result;
}

or one liner

private String getStringIfObjectIsPresent(Optional<Object> object) {
   return your-map-function(object.orElseThrow(MyCustomException::new));
}
iamiddy
  • 3,015
  • 3
  • 30
  • 33
  • first solution is the cleanest and most readable of all solutions in this entire thread – Benas Sep 27 '22 at 13:55
1

The example u put is not a good example. Optional shouldn't be sent as parameter into another function. Good practice is always sending non-null parameter into function. So that we know always that the input will not be null. This can decrease our code uncertainty.

Ryan Zheng
  • 73
  • 1
  • 5