Search code examples
javaoption-type

Getting a value from an inside an Optional with isPresent()


I have a User and associated time-limited Roles. I want to know if a User has a particular UserRole and it is unexpired. I can turn the user's roles into a stream, filter() it and findFirst(), giving me an Optional.

Role

public class Role {
    private UserRole role;
    private Date expiry;
    
    public boolean isUnexpired () {
        return (expiry == null) ? true : expiry.after(new Date());
    }
}

User

public class User {
  //...
  private Collection<Role> roles

  public boolean hasRole (UserRole userRole) {
    return roles.stream()
      .filter(r -> r.getRole().equals(userRole))
      .findFirst()
      .ifPresent(ur -> {  /* ... herein the problem ... */ ur.isUnexpired(); } );
  }
}

The problem in that last line is that ifPresent() has a void signature; as such, I can't return ur.isUnexpired() from it. Whatever I put in the lambda expression or anonymous inner class at that point can't do anything meaningfully with the value it finds.

I tried declaring a boolean before filtering the stream, and assigning it, but get the (code validation) error: local variables referenced from a lambda expression must be final or effectively final.

(I know, there's a bit more to handle when it's not present; if I can sort this, I can swap to ifPresentOrElse().)

I could do the following:

  public boolean hasRole (UserRole userRole) {
    Optional<Role> o = roles.stream()
      .filter(r -> r.getRole().equals(userRole))
      .findFirst();
    return o.isPresent() ? o.get().isUnexpired() : false;
  }

However, I would rather do it with a cleaner, chained function.

Is there some way to extract and use my isUnexpired() boolean with a chained function? Or must I assign the Optional then operate on it separately?


Solution

  • If you only care if a user has ANY unexpired entry of a UserRole in Collection<Role> roles, matching with a predicate using anyMatch() is a clearer and more concise approach:

    public boolean hasRole (UserRole userRole) {
       return roles.stream()
           .anyMatch(r -> r.role.equals(userRole) && r.isUnexpired());
    }
    
    

    This approach using anyMatch() will be clear to future readers of your code without any ambiguity.

    Why not use the stream/filter/findFirst approach? At minimum, it's possible based on just the info in the OP to have multiple entries in the private Collection<Role> roles with same UseRole and different expiry dates. findFirst() will be produce indeterminate results unless you do something to ensure ordering of the collection (Collection itself has no guarantees on order) or do the expiration check in the filter. Even with check in filter, I argue anyMatch() is less open for question on your intent.