Чистый способ избежать условий

Question

У меня есть такой код:

 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));
}

Есть ли чистый способ избежатьIFS?Я хочу, чтобы мой код решал, должен ли он использовать аргумент -I или -T.Я обработал ситуации, когда args не имеет ничего, я просто упростил код для StackOverflow.Я использую методы findByTitle и findByIsbn много раз в своем коде, поэтому я не уверен, что здесь подойдет другой метод.

Answer 1

Вы можете избавиться от if с помощью двух комбинированных шаблонов.Я также думаю, что if в первом блоке несколько не на том же уровне абстракции, что и код вокруг него.Другая проблема заключается в том, что это не следует принципу открытого закрытия.Допустим, вам нужен другой тип параметра поиска, например, регистрационный номер -R или любой другой.Вам нужно дотронуться до оператора if, и если он становится все больше и больше, вы можете что-то сломать.

, поэтому давайте посмотрим, как может выглядеть хороший код без if

args.put("-T", "Tom Sawyer");
args.put("-I", "1112223334");
Book book = libraryService.findBook(args);

Ноэто, очевидно, не работает как это из коробки.Но вы можете сделать что-то подобное с шаблоном стратегии.

IBookFindStrategy {
   Book findBook(string param)
}

class IsbnFindStrategy : IBookFindStrategy {
   Book findBook(string param) {
      // your repocall
   }
}

class NameFindStrategy : IBookFindStrategy {
   Book findBook(string param) {
      // your repocall
   }
}

Теперь вам просто нужно преобразовать параметры в другом месте и инициализировать правильную стратегию на фабрике.Фабрика может хранить параметры в Hashmap и вызывать их с параметром -T, который даст вам NameFindStrategy.Примерно так

class StrategyFactory {
      Hashtable<String, IBookFindStrategy > strategies;

      public StrategyFactory() {
          strategies = new HashMap<String, IBookFindStrategy >();
          strategies.put("-T", new NameFindStrategy());
          strategies.put("-I", new NameIsbnFindStrategy());
      }

      public IBookFindStrategy GetStrategy(string param) {
          return strategies.get(param);
      }
}

И, наконец, ваш main будет выглядеть примерно так:

StrategyFactory factory = new StrategyFactory();
IBookFindStrategy bookFinder = factory.getStrategy(args);
Book book = bookFinder.findBook(args);

Я не на dev-машине, а моя java немного ржавая, поэтому ямне немного лень записывать все, но я надеюсь, что вы поняли концепцию.

Answer 2

Вместо передачи аргументов в хранилище, передайте ему полное значение Predicate:

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

Затем вы можете вызвать его как:

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

Answer 3

Как представляется, код выглядит в его простейшей и, вероятно, лучшей форме.

Однако вы можете использовать отображение «поиск книг» для удаления явных блоков if.Вот одна версия, в которой используются поставщики:

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

. Затем ее можно использовать в коротком потоке всех возможных ключей:

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

. Приведенное выше вернет null, если ни -T ни -I на карте.

Answer 4

Ваш код выглядит довольно хорошо, по крайней мере для меня.

Ваш оператор if не нуждается в удалении.Они не дублируются, поэтому держите их там.

С другой стороны, я нашел некоторый дублирующий код в методах findBy:

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));
}

Вы можете написать новый метод с именем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));
}

А затем findByIsbn и findByTitle вызов findBy:

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

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