Spring Boot - throw exception or indicate item is not found

Viewed 1781

I've written two methods, findById searches for an item in the DB and throws an exception if the item is not found :

public Url findById(final Long id){
        return urlRepository.findById(id)
                .orElseThrow(() -> new ShortUrlNotFoundException("URL not found for the given ID"));
}

The second method, findByShortUrl searches for an item in the DB and uses the JPA method findByShortUrlIs which returns a List of size 1 if the item is found, there should never be more than 1 item in the DB for a given shortUrl :

public Optional<String> findByShortUrl(final String shortUrl){

    List<Url> urlList = urlRepository.findByShortUrlIs(shortUrl);
    if(urlList.isEmpty()){
        return Optional.empty();
    }
    else {
        return Optional.of(urlList.get(0).getLongUrl());
    }
}

I like the pattern of using a ShortUrlNotFoundException if an item is not found. Should I use it also in findByShortUrl ? Then, findByShortUrl becomes:

public Optional<String> findByShortUrl(final String shortUrl){

    List<Url> urlList = urlRepository.findByShortUrlIs(shortUrl);
    if(urlList.isEmpty()){
        throw new ShortUrlNotFoundException("URL not found for the given ID")
    }
    else {
        return Optional.of(urlList.get(0).getLongUrl());
    }
}
2 Answers

Personally speaking, I would never use exception mechanism for both cases. Exceptions are designed for unexpected situations that aren't covered by conventional business logic. For example, findById should throw an exception for the search if and only if you do not expect that item in the database. The same story for the findByShortUrl. As far as I understand it's a normal case not to have a object in db, thus it should be expected case to have empty result. On other hand, you've mentioned that "there should never be more than 1 item in the DB", and that's the perfect case for an exception! If you see that return result set has more than 2 objects, so go and thrown an excpetion.

Why not using findFirst as this:

Optional<Url> findFirstByShortUrlIs(String shortUrl);

and then, you call:

public Optional<String> findByShortUrl(final String shortUrl){
        return urlRepository.findFirstByShortUrlIs(shortUrl)
                .map(Url::getLongUrl)
                .map(Optional::of)
                .orElseThrow(() -> new ShortUrlNotFoundException("URL not found for the given ID"));
}
Related