Best practice Optional

Какой из подходов best practice?

Optional<ElementEntity> element = repositoty.findByName();
element.ifPresent(item -> {
    if (!item.getId().equals(elementDTO.getId()) {
        throw new ElementExistException();
    }
});

VS

ElementEntity element = repositoty.findByName().orElse(null); 
if (element != null && !element.getId().equals(elementDTO.getId()) {
    throw new ElementExistException();
}

Ответы (1 шт):

Автор решения: Alex Rudenko

Второй пример, где Optional приводится к null, чтобы в следующей строке выполнить проверку на этот null, явно хуже, но и первый не идеален.

Во-первых, в методе findByName() нет входного аргумента. Во-вторых, проверку наличия имени (которое, видимо, должно быть уникальным) для другого идентификатора следует реализовать на уровне репозитория, например, в случае JPA репозитория в него следует добавить метод с таким именем:

// repository
Optional<ElementEntity> findByNameAndIdNot(String name, Object id);

который автоматически проверит оба поля.

Тогда при вызове код упростится:

// service
repository.findByNameAndIdNot(name, elementDto.getId())
    .ifPresent(x -> throw new ElementAlreadyExistsException());

Также можно использовать методы Optional::filter и/или Optional::map, которые выполняются, если существует соответствующее не-null значение:

Optional<ElementEntity> element = repository.findByName(name);
Object dtoId = elementDTO.getId();
element
    .map(ElementEntity::getId) //Optional<IdClass>
    .filter(dtoId::equals)     //Optional<IdClass>
    .ifPresent(x -> throw new ElementAlreadyExistsException());

// или без промежуточного вызова map
element
    .filter(x -> dtoId.equals(x.getId())) // здесь x NOT null
    .ifPresent(x -> throw new ElementAlreadyExistsException());
→ Ссылка