Poprawność użycia Optionala

Ostatnio zacząłem ogarniać nowe featury z javy 8. Naklepałem podstawowe operacje na repozytorium. W związku z tym mam pytanie czy to tak powinno podręcznikowo wyglądać. Czy powinienem jeszcze opakować w Optional List<User> users? Co w prostym kodzie mogę jeszcze poprawić?

@Repository
public class UserDaoImpl implements UserDao {

    private List<User> users;

    public UserDaoImpl() {

        users = new ArrayList<>();
        users.add(new User(1, "Michal", 20, new BigDecimal(18000)));
        users.add(new User(2, "Weronika", 22, new BigDecimal(15000)));
        users.add(new User(3, "Radek", 22, new BigDecimal(10000)));
        users.add(new User(4, "Artur", 19, new BigDecimal(2000)));
        users.add(new User(5, "Armando", 17, new BigDecimal(1500)));
    }

    @Override
    public Optional<User> findById(long id) {
        return users.stream()
                .filter(u -> u.getId() == id)
                .findFirst();
    }

    @Override
    public Optional<User> findByName(String name) {
        return users.stream()
                .filter(u -> u.getName().equals(name))
                .findFirst();
    }

    @Override
    public void saveUser(User user) {
        users.add(user);
    }

    @Override
    public void updateUser(User user) {
        Optional<User> found = findById(user.getId());
        if (found.isPresent()) {
            User foundUser = found.get();
            foundUser.setName(user.getName());
            foundUser.setAge(user.getAge());
            foundUser.setSalary(user.getSalary());
        }
    }

    @Override
    public void deleteUserById(long id) {
        Optional<User> found = findById(id);
        found.ifPresent(user -> users.remove(user));
    }

    @Override
    public void deleteAllUsers() {
        users.clear();
    }

    @Override
    public List<User> findAllUsers() {
        return users.stream()
                .sorted(Comparator.comparing(User::getName).thenComparing(User::getAge).thenComparing(User::getSalary))
                .collect(Collectors.toList());
    }

    @Override
    public boolean isUserExist(User user) {
        return users.contains(user);
    }
}

W metodzie updateUser mógłbyś zastosować operator ifPresent i np. referencję na metodę zamiast isPresent i get tak jak to zrobiłeś w deleteUserById

1. Tą listę możesz opakować w Optional tylko jeśli to się może wywalić i chcesz tak syngalizować błąd. U ciebie to nie ma miejsca więc zwrócenie pustej listy jest ok.
2. do findBy... możesz zrobić osobną metodę która przyjmuje predykat do filtru jako argument i tą metodę wywołać w tych twoich findByCośtam. Widzisz przecież że masz tam teraz copypaste ;]

Gdzieś na Voxxed był wykład o Optionalach, bodajże z zeszłego roku. Jak znajdę to podrzucę ;)

Co do Optional - to nie ma sensu wrzucać jak masz List.
Natomiast masz 3 metody: saveUser, updateUser,deleteUserById które generalnie mogą nie pójść dobrze (użytkownik o danym id już istnieje np.).
I tu są różne opcje co z tym zrobić - ale dla twojego kodu najbardziej by pasowało wywalenie Runtimowego (!) exceptiona,

No i druga smutna rzecz - ponieważ User jest mutowalny (czyli ma setery) to ten kod nigdy nie będzie bezpieczny - pobierasz Usera po id - a potem mu zmieniasz name itp. ..
Obiekt zmienia się w twojej bazie. (tak samo jest zresztą w JPA). Wcale nie musisz używać update.