Clean way to avoid conditions

Question

I've got a code like this:

 Map<String, String> args = new HashMap<>();

    args.put("-T", "Tom Sawyer");
//        args.put("-I", "1112223334");

    if (args.containsKey("-T")) {
        Book book = libraryService.findBookByTitle(args.get("-T"));
    } else {
        Book book = libraryService.findBookByIsbn(args.get("-I"));                       
    }  

LibraryService:

public class LibraryService {

    private final BookRepository bookRepository = new BookRepository();

    public Book findBookByTitle(String title) {
        return bookRepository.findByTitle(title);
    }

    public Book findBookByIsbn(String isbn) {
        return bookRepository.findByIsbn(isbn);
    }

BookRepository:

public class BookRepository {

 private List<Book> books = new ArrayList<>();

  public Book findByIsbn(String isbn) {
        return books.stream()
            .filter(s -> s.getIsbn().equals(isbn))
            .findFirst()
            .orElseThrow(() -> new RuntimeException(NO_BOOKS_FOUND));
}

    public Book findByTitle(String title) {
        return books.stream()
            .filter(s -> s.getTitle().equals(title))
            .findFirst()
            .orElseThrow(() -> new RuntimeException(NO_BOOKS_FOUND));
}

Is there a clean way to avoid ifs? I want my code to decide whether it has to use argument -I or -T. I handled situations when args don't have any of it, I just simplified code for StackOverflow. I use methods findByTitle and findByIsbn many times in my code, so I'm not sure if another method would suit here.

Answer 1

Instead of passing arguments to repository, pass a complete Predicate to it:

public class findOneByPredicate(Predicate<Book> filter) {
        return books.stream()
            .filter(filter)
            .findFirst()
            .orElseThrow(() -> new RuntimeException(NO_BOOKS_FOUND));
}

Then you can call it like:

findOneByPredicte(b -> b.getIsbn().equals("ISBN"));

Answer 2

As it is, the code seems to be at its simplest and probably best form.

However, you could use a mapping of "book finders" to remove the explicit if blocks. Here's one version that uses suppliers:

Map<String, Function<String, Book>> resolvers = new HashMap<>();
resolvers.put("-T", libraryService::findBookByTitle);
resolvers.put("-I", libraryService::findBookByIsbn);

That can then be used in a short stream of all possible keys:

Book book = Stream.of("-T", "-I").filter(args::containsKey)
                  .findFirst()
                  .map(key -> resolvers.get(key).apply(args.get(key)))
                  .orElse(null);

The above will return null if neither -T nor -I is in the map.

Answer 3

Your code looks pretty fine, at least to me.

Your if statements don't need removing. They are not duplicated, so keep them there.

On the other hand, I did find some duplicate code in the findBy methods:

public Book findByIsbn(String isbn) {
    return books.stream()
        .filter(s -> s.getIsbn().equals(isbn))
        .findFirst()
        .orElseThrow(() -> new RuntimeException(NO_BOOKS_FOUND));
}

public Book findByTitle(String title) {
    return books.stream()
        .filter(s -> s.getTitle().equals(title))
        .findFirst()
        .orElseThrow(() -> new RuntimeException(NO_BOOKS_FOUND));
}

You can write a new method called findBy:

private Book findBy<T>(Function<Book, T> selector, T value) {
    return books.stream()
        .filter(s -> selector.apply(s).equals(value))
        .findFirst()
        .orElseThrow(() -> new RuntimeException(NO_BOOKS_FOUND));
}

And then have findByIsbn and findByTitle call findBy:

public Book findByIsbn(String isbn) {
    return findBy(Book::getIsbn, isbn);
}

public Book findByTitle(String title) {
    return findBy(Book::getTitle, title);
}