0

I have a service in Springboot and I want to get an optional entity and to check if is not null to do something after.

User repository class

public interface UserRepository extends JpaRepository<UserEntity, Long> {

    Optional<UserEntity> findByLastName(String lastName);
}

Role repository class

public interface RoleRepository extends JpaRepository<RoleEntity, Long> {
    Optional<RoleEntity> findByName(String name);
}

How can I write properly this code below?

 public UserEntityDto addRoleToUser(String username, String rolename) {
        Optional<UserEntity> usrDb = userRep.findByLastName(username);
        Optional<RoleEntity> roleDb = roleRep.findByName(rolename);
        UserEntity userEntity = usrDb.orElse(null);
        RoleEntity roleEntity = roleDb.orElse(null);
        if (userEntity != null && roleEntity != null) {
            userEntity.getRoles().add(roleEntity);
        }

        return userEntityMapper.fromUserEntity(userEntity);
    }

3 Answers 3

1

There's no nice way of combining optionals in situations like this.

You could possibly look at Optional.ifPresent if you like lambdas;

usrDb.ifPresent(u -> {
  roleDb.ifPresent(r -> u.getRoles().add(r));
}

Alternatively use Optional.isPresent

if (usrDb.isPresent() && roleDb.isPresent()) {
  usrDb.get().getRoles().add(roleDb.get())
}
1

I would do herror handling first:

public UserEntityDto addRoleToUser(String username, String rolename) {

    // You should create you own runtime exceptions and handle it on a "Controller Advice" class
    UserEntity user = userRep.findByLastName(username)
            .orElseThrow(() -> new RuntimeException("User does not exists"));

    RoleEntity role = roleRep.findByName(rolename)
            .orElseThrow(() -> new RuntimeException("Role does not exists"));

    user.getRoles().add(role);

    return userEntityMapper.fromUserEntity(userEntity);
}

If you want to keep the null logic you can do something like this, but it's way harder to understand:

public UserEntityDto addRoleToUser(String username, String rolename) {
    Optional<UserEntity> usrDb = userRepository.findByLastName(username);
    Optional<RoleEntity> roleDb = roleRepository.findByName(rolename);

    return usrDb.flatMap(
                    userEntity -> roleDb.map(
                            roleEntity -> {
                                userEntity.getRoles().add(roleEntity);
                                return userEntityMapper.fromUserEntity(userEntity);
                            }
                    )
            )
            .orElse(null);
}
0

I think you need to consider negative scenario while writing method. Such as

  1. There may be no user with the username you provided.
  2. There may be no userRole with the role name as you argument. In those negative scenario you should through exception (e.g. NoSuchElementException or IllegalArgumentException) instead of digesting null. If I were designing that method I would write it like
   public UserEntityDto addRoleToUser(String username, String rolename) throws NoSuchElementException{
    Optional<UserEntity> usrDb = userRep.findByLastName(username);
    if (!usrDb.isPresent()) throw new NoSuchElementException("User not found with this username");
    Optional<RoleEntity> roleDb = roleRep.findByName(rolename);
    if (!roleDb.isPresent()) throw new NoSuchElementException("User role not found with this role name");
    usrDb.get().getRoles().add(roleDb.get());
    return userEntityMapper.fromUserEntity(usrDb.get());
}

Not the answer you're looking for? Browse other questions tagged or ask your own question.