Fetching Optional based on condition Java

Question

I am trying to get value from multiple Optional based on priority and condition Lets say in following two set of optional the activitys that can be return are Walking and Swimming. If there is any activity in either of the optional and if swimming is there, then swimming should get preference else walking. If there are no activities then this will return empty. I managed to write it but there are too many conditions and wanted to see if there is a smart way of avoiding so many conditions

public Optional<Activity> getActivity(){
    Optional<Activity> activityWhenSunShines= getActivityWhenSunShiningForUser(u);
    Optional<Activity> activityWhenDayIsGood= getActivityWhenDayIsGoodForUser(u);
    if(activityWhenSunShines.isPresent() && Activity.SWIMMING == activityWhenSunShines.get()){
      return activityWhenSunShines;
    }else if(activityWhenDayIsGood.isPresent() && Activity.SWIMMING == activityWhenDayIsGood.get()){
      return activityWhenDayIsGood;
    }else if(activityWhenSunShines.isPresent()){
      return activityWhenSunShines;
    }else if(activityWhenDayIsGood.isPresent()){
      return activityWhenSunShines;
    }else{
      return Optional.empty();
    }
  }

It looks like your 2nd last case should return `activityWhenDayIsGood`, not `activityWhenSunShines` — Michael, Jun 14 '21 at 14:45
Do you just want to return the Optional which is present? Is there some priority to these conditions? — aksappy, Jun 14 '21 at 15:14

score 1 · Accepted Answer · answered Jun 14 '21 at 14:44

1

This code

activityWhenSunShines.isPresent() && Activity.SWIMMING == activityWhenSunShines.get()

can be converted to a more functional style, without isPresent followed by get

activityWhenSunShines.map(a -> a == Activity.SWIMMING).orElse(false)

The last 3 cases can be replaced with Optional.or (added in Java 9).

That gives you:

Optional<Activity> activityWhenSunShines = getActivityWhenSunShiningForUser(u);
Optional<Activity> activityWhenDayIsGood = getActivityWhenDayIsGoodForUser(u);

if(activityWhenSunShines.map(a -> a == Activity.SWIMMING).orElse(false)){
  return activityWhenSunShines;
} else if(activityWhenDayIsGood.map(a -> a == Activity.SWIMMING).orElse(false)){
  return activityWhenDayIsGood;
}
return activityWhenSunShines.or(() -> activityWhenDayIsGood);

answered Jun 14 '21 at 14:44

Michael

41,989
11
82
128

I believe you might have meant `.filter` and not `.map`. – Naman Jun 14 '21 at 17:11
@Naman No, I meant what I said. https://ideone.com/gyovYX `filter` followed by `isPresent` would also work, but it's slightly longer and I believe doesn't convey the intent as well. The map version says "*take the item in the optional - is it SWIMMING? if there is no item, then it is not SWIMMING*". The filter version reads like "*remove the item from the optional if it's SWIMMING, and then if there's no item at the end, the item must have been SWIMMING*". Accomplishes the same goal but with less clear semantics. – Michael Jun 14 '21 at 18:09
I differ and I know that this might step into a grey area of discussion which primarily ends more with opinion based ideologies. But the intention with `if(activityWhenSunShines.map(a -> a == Activity.SWIMMING).orElse(false)){ return activityWhenSunShines;}` is not to `map`, but to return based on the presence. Further that is justified with the expectation of a `Predicate` versus `Function` as arguments to those corresponding APIs. But sure, as long as it does the job... – Naman Jun 15 '21 at 14:43
I will mark this as answer – Sandeep Nair Jun 16 '21 at 06:17

score 1 · Answer 2 · answered Jun 14 '21 at 15:14

Patient: Doctor, doctor! It hurts when I smash this hammer against my face!

Doctor: Okay. Stop doing that then.

Optional (in the java ecosystem, at least) is mostly bad. It's fine for stream terminal return values and not much else.

The much better alternative is to just have a data structural setup where neither null nor Optional are relevant. The next-best alternative is to use null (and add some nullity annotations if you prefer compile-time checks). A very distant crappy third solution is the dreaded Optional.

For example, here, why not have an Activity.NOTHING, and spec gAWSSFU to never return null?

Here's what your code would look like if you did that:

Activity activityWhenSunShines = getActivityWhenSunShiningForUser(u);
Activity activityWhenDayIsGood = getActivityWhenDayIsGoodForUser(u);

return
  (activityWhenSunShines == Activity.SWIMMING || activityWhenDayIsGood) ? Activity.SWIMMING :
  activityWhenSunShines != Activity.NOTHING ? activityWhenSunShines :
  activityWhenDayIsGood;

If instead of nothing you prefer null here, it's the exact same code, just replace Activity.NOTHING with null and you're good to go.

Optional doesn't compose (generics has co/contra/in/legacy-variance, in order to be composable. The nullity dimension introduced by Optional doesn't play that game; you e.g. can write a method that takes in a list of Number or some subclass thereof. You can't write a (type-safe) method that takes a list of either String or Optional<String>. Optional is also entirely backwards incompatible (e.g. map.get(x) does not return Optional<V> and never will, given that java doesn't break backwards compatibility, especially not of such an oft-used method). If you want compile-time-checked null safety, have a look at null annotation frameworks, such as the one baked into your IDE (both Eclipse and IntelliJ have their own system, fully supported with compile time checks), or use e.g. checker framework's, which is more flexible than what eclipse and intellij offer.

I will upvote this as its a nice solution but could not change what I have. I agree having nothing is ths smartest thing to do, to have a final fallback and never have null or Optional.empty.. Edit - I upvoted but someone has downvoted so its 0 :) — Sandeep Nair, Jun 16 '21 at 06:20

Fetching Optional based on condition Java

2 Answers2