1
\$\begingroup\$

I have a MongoRepository:

@Repository
public interface GooglePlayGameRepository extends MongoRepository<GooglePlayGame, String> {

    Optional<GooglePlayGame> findByTitle(String title);

    List<GooglePlayGame> findByTitleContainsIgnoreCase(String title);

    @Aggregation("{$sample: {size: ?0} }")
    List<GooglePlayGame> findRandomGames(Long number);

}

And service:

@Service
public class GooglePlayGameService {

    private final GooglePlayGameRepository googlePlayGameRepository;

    public GooglePlayGameService(GooglePlayGameRepository googlePlayGameRepository) {
        this.googlePlayGameRepository = googlePlayGameRepository;
    }

    public void saveToLibrary(GooglePlayGame googlePlayGame) {
        googlePlayGameRepository.save(googlePlayGame);
    }

    public GooglePlayGame getGameByTitle(String title) throws NoSuchGooglePlayGameFoundException {
        Optional<GooglePlayGame> googlePlayGame = googlePlayGameRepository.findByTitle(title);
        return googlePlayGame.orElseThrow(
                () -> new NoSuchGooglePlayGameFoundException("Game was not found: " + title)
        );
    }

    public List<GooglePlayGame> findByTitle(String title) {
        List<GooglePlayGame> googlePlayGames = googlePlayGameRepository.findByTitleContainsIgnoreCase(title);
        return googlePlayGames.isEmpty()
                ? Collections.emptyList()
                : googlePlayGames;
    }

    public Long getLibrarySize() {
        return googlePlayGameRepository.count();
    }

    public List<GooglePlayGame> getRandomGames(Long number) {
        return googlePlayGameRepository.findRandomGames(number);
    }

}

What do you think about methods getGameByTitle (returns exact match) and findByTitle (looking for similar matches by title)? Is it good practice to handle Optional in Service layer and just throws Exception? Or should I use Optional as a return type in Service layer too and handle it in other places which will use this service?

About findByTitle: is it necessary to use this construction?

googlePlayGames.isEmpty()
          ? Collections.emptyList()
          : googlePlayGames;

Maybe I should just check it in code like this (not in the service):

if (googlePlayGames.isEmpty()){
   log.error("Game {} doesn't exist in library", title);
}

I have another variant for these methods:

public GooglePlayGame getGameByTitle(String title) throws NoSuchGooglePlayGameFoundException {
    return findByTitle(title).stream()
             .findFirst()
             .orElseThrow(NoSuchGooglePlayGameFoundException::new);
 }

public List<GooglePlayGame> findByTitle(String title) {
     List<GooglePlayGame> games = googlePlayGameRepository.findByTitleContainsIgnoreCase(title);
     if (!games.isEmpty()) {
         return games;
     }
     return Collections.emptyList();
 }

Which one is better?

\$\endgroup\$
1
\$\begingroup\$
  1. You don't need to make additional save method since save method already exist.
public void saveToLibrary(GooglePlayGame googlePlayGame) {
        googlePlayGameRepository.save(googlePlayGame);
    }
  1. You have two methods one called findByTitle(String title) and findByTitleContainsIgnoreCase

To make other programmer understand whats going on maybe renaming findByTitleContainsIgnoreCase to findListOfGamesByTitlesince you are returning List and also it can be optional since games might not be there at all.

In your service call findListOfGamesByTitle just pass String (title.equalsIgnoreCase())

  1. Why are you throwing exception at method level and also inside the body in getGameByTitle , do you want to pass that exception further ? there is no point doing it.

  2. Yes, you should handle optional in the service layer because next layer is the constructor.

The sentence and the place you mentioned is not understood so please fix it a bit i am referring to the following -> Maybe I should just check it in code like this (not in the service):

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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