Как избежать кода дерева if-else? - PullRequest
0 голосов
/ 27 апреля 2018

У меня есть контроллер отдыха, который включает метод поиска книг в зависимости от параметров "title" и "author".

Не могли бы вы дать мне подсказку, как избавиться от конструкции if-else? Сейчас это не сложно, но в будущем число параметров может увеличиться, что приведет к путанице.

    @GetMapping
    public ResponseEntity<List<Book>> searchBooksByTitleAndAuthor(
            @RequestParam(value = "title", required = false) String title,
            @RequestParam(value = "author", required = false) String author) {

        if (title == null && author == null) {
            log.info("Empty request");
            return new ResponseEntity<>(HttpStatus.NO_CONTENT);
        } else if (title != null && author == null) {
            log.info("Retrieving books by title");
            List<Book> books = bookService.getBooksByTitle(title);
            if (books.isEmpty()) {
                log.info("No books with this title");
                return new ResponseEntity<>(HttpStatus.NO_CONTENT);
            }
            return new ResponseEntity<>(books, HttpStatus.OK);
        } else if (author != null && title == null) {
            List<Book> books = bookService.getBooksByTitle(title);
            if (books.isEmpty()) {
                log.info("No books with this title");
                return new ResponseEntity<>(HttpStatus.NO_CONTENT);
            }
            return new ResponseEntity<>(books, HttpStatus.OK);
        } else {
            List<Book> books = bookService.getBooksByTitleAndAuthor(title, author);
            if (books.isEmpty()) {
                log.info("No books with matching this title and author");
                return new ResponseEntity<>(HttpStatus.NO_CONTENT);
            }
            return new ResponseEntity<>(books, HttpStatus.OK);
        }

    }

Ответы [ 6 ]

0 голосов
/ 27 апреля 2018

Вот идея. Код может потребовать некоторой работы, в частности, я не знаю, можете ли вы использовать статическую карту, но в Spring у вас, вероятно, все равно есть синглтон.

private static long toKey(Object ... args) {
    long key = 0L;
    for(int i = 0; i < args.length; i++) {
        if(args[i] != null) {
            key |= (1L << i);
        }
    }
    return key;
}

private static interface BookFinder {
    ResponseEntity<List<Book>> search(String title, String author);
}

private static Map<Long, BookFinder> _keyToFinderMap = new HashMap<>();

static {
    _keyToFinderMap.put(toKey(null, null), new BookFinder() {
        public ResponseEntity<List<Book>> search(String title, String author) {
            log.info("Empty request");
            return new ResponseEntity<>(HttpStatus.NO_CONTENT);
        }
    });
    _keyToFinderMap.put(toKey("", null), new BookFinder() {
        public ResponseEntity<List<Book>> search(String title, String author) {
            log.info("Retrieving books by title");
            List<Book> books = bookService.getBooksByTitle(title);
            if (books.isEmpty()) {
                log.info("No books with this title");
                return new ResponseEntity<>(HttpStatus.NO_CONTENT);
            }
            return new ResponseEntity<>(books, HttpStatus.OK);
        }
    });
    // Other cases
};

@GetMapping
public ResponseEntity<List<Book>> searchBooksByTitleAndAuthor(
        @RequestParam(value = "title", required = false) String title,
        @RequestParam(value = "author", required = false) String author) {
    return _keyToFinderMap.get(toKey(title, author)).search(title, author);
}

Если вы добавляете новый аргумент, вы просто добавляете новый искатель книг. Нет, если заявления. Я бы переместил NO_CONTENT на searchBooksByTitleAndAuthor, но я не хочу менять ваши операторы регистрации. В противном случае это немного упростит поиск.

Метод toKey, конечно, может быть изменен, реализация не важна. Он должен просто сопоставить комбинации ввода с уникальными ключами. Предлагаемый подход обрабатывает до 64 аргументов, которые могут быть нулевыми / ненулевыми эффективно.

0 голосов
/ 27 апреля 2018

Я бы порекомендовал Спецификация или Querydsl .
Он основан на доменном дизайне / модели, и реализация основывается на критериях JPA, которые обеспечивают гибкость в построении запросов.

Вот пример (не проверено, но вы должны получить общее представление).

Характеристики книги

Класс фабрики для динамического создания спецификаций.

public class BookSpecifications {

    private BookSpecifications(){
    }

    public static Specification<Book> withAuthor(String author) {
        return new Specification<Book>() {
            public Predicate toPredicate(Root<Book> root, CriteriaQuery<?> query, CriteriaBuilder cb) {
                return cb.equal(root.get("author"), author);
            }
        };
    }

    public static Specification<Book> withTitle(String title) {
        return new Specification<Book>() {
            public Predicate toPredicate(Root<Book> root, CriteriaQuery<?> query, CriteriaBuilder cb) {
                return cb.equal(root.get("title"), title);
            }
        };
    }
}

Обратите внимание, что использование постоянных значений, которые были сгенерированы во время компиляции из классов Entity, является более подходящим подходом. Yo может ссылаться на атрибуты сущности с помощью Book.author или Book.title вместо использования некоторых String, не проверенных во время компиляции, с реальной моделью сущности, таких как «автор» или «заголовок».

BookController

На стороне контроллера вы должны избегать сокращения до минимума логики и отдавать предпочтение делегированию обработки классу обслуживания, который создаст требуемую спецификацию и передаст ее в хранилище.

public class BookController {
    @GetMapping
    public ResponseEntity<List<Book>> searchBooksByTitleAndAuthor(
            @RequestParam(value = "title", required = false) String title,
            @RequestParam(value = "author", required = false) String author) {


        List<Book> books = bookService.findAll(title, author);

        if (books.isEmpty()) {
            log.info("No books with this specification ");
            return new ResponseEntity<>(HttpStatus.NO_CONTENT);
        }
        return new ResponseEntity<>(books, HttpStatus.OK);

    }
}

BookService

Вся логика здесь (модульные тесты должны быть сосредоточены здесь).

public class BookService {

    private BookRepository bookRepository;

    public BookService(BookRepository bookRepository){
         this.bookRepository = bookRepository;
    }

    public List<Book> findAll(String title, String author) {

       if (Stream.of(title, author)
                 .filter(s-> s == null || s.equals(""))
                 .count() == 0) {
            return new ArrayList<>();
        }

        Specification<Book> specification = createSpecification(specification, title, () -> BookSpecifications.withTitle(title));
        specification = createSpecification(specification, author, () -> BookSpecifications.withAuthor(author));
        List<Book> books = bookRepository.findAll(specification);
        return books;

    }

    Specification<Book> createSpecification(Specification<Book> currentSpecification, String arg, Callable<Specification<Book>> callable) {

        // no valued parameter so we return
        if (arg == null) {
            return currentSpecification;
        }

        try {

           // Specification instance already created : reuse it
            if (currentSpecification != null) { 
                return currentSpecification.and(callable.call());
            }

           // Specification instance not created yes : create a new one
            return callable.call();
        } catch (Exception e) {
            // handle the exception... if any
        }

    }
}

Добавить новый атрибут сущности в поиск очень просто.
Добавьте параметр в метод и в поток, который проверяет, заполнен ли хотя бы критерий поиска, а затем добавьте новый вызов в цепочку createSpecification():

// existing
Specification<Book> specification = createSpecification(specification, title, () -> BookSpecifications.withTitle(title));
specification = createSpecification(specification, author, () -> BookSpecifications.withAuthor(author));
// change below !
specification = createSpecification(specification, anotherColumn, () -> BookSpecifications.withAnotherColumn(anotherColumn));

BookRepository

Последний шаг: сделайте ваш BookRepository интерфейс расширяемым JpaSpecificationExecutor, чтобы иметь возможность вызывать Repository методы, принимающие Specification<T> параметры, такие как:

List<T> findAll(@Nullable Specification<T> spec);

Это должно быть хорошо:

@Repository
public interface BookRepository extends JpaRepository<Book, Long> , JpaSpecificationExecutor<Book>  {    
}

Обратите внимание, что если может быть запрошено много атрибутов объекта, использование более динамичного подхода может быть интересным:

public class BookSpecifications {

    private BookSpecifications(){
    }

    public static Specification<Book> withAttribute(String name, T value) {
        return new Specification<Book>() {
            public Predicate toPredicate(Root<Book> root, CriteriaQuery<?> query, CriteriaBuilder cb) {
                return cb.equal(root.get(name), value);
            }
        };
    }

}

Но у него есть и недостаток для отсрочки обнаружения ошибок кодирования.
Действительно, ошибки в спецификации здания (например, тип значения не совместимы) могут быть обнаружены только во время выполнения.

0 голосов
/ 27 апреля 2018

Небольшое вложение может помочь. Это сведет к минимуму повторяющиеся тесты. Какой-то код:

if (StringUtils.isNotBlank(blam) || StringUtils.isNotBlank(kapow))
{
    if (StringUtils.isBlank(blam))
    {
        // kapow is not blank.
    }
    else if (StringUtils.isBlank(kapow))
    {
        // blam is not blank.
    }
    else
    {
        // neither kapow nor blam is blank.
    }
}
else
{
   // both empty.  error.
}

Мне нравится ответ Брайана, но я бы никогда не рекомендовал использовать Hibernate. MyBatis также поддерживает условные предложения where.

0 голосов
/ 27 апреля 2018

Поскольку ваш план состоит в том, чтобы в конечном итоге поддерживать больше параметров, лучше всего было бы рассмотреть класс Hibernate Criteria класс . Это позволяет динамически создавать запросы. Он не избежит операторов if, но позволит избежать операторов else и упростит поддержку новых параметров. На вашем хранилище / уровень DAO:

Criteria criteria = session.createCriteria(Book.class);
if (author != null && !author.isEmpty()) {
    criteria.add(Restriction.eq("author", author));
}
if (title != null && !title.isEmpty()) {
    criteria.add(Restriction.eq("title", title));
}
criteria.addOrder(Order.asc("publishDate"));
return (List<Book>) criteria.list();

Это имеет некоторые заметные преимущества:

  1. Для поддержки нового параметра вам просто нужно добавить свой параметр в контроллер, затем передать параметр в хранилище и добавить параметр в этот блок.

  2. Вы можете в конечном итоге сделать свой вид настраиваемым, например ::

    ?sort=title:asc&author=Bobby%20Tables

Однако есть некоторые недостатки, наиболее очевидным из которых является то, что от ваших строковых значений зависит значение строки. Если имя вашего свойства изменится (обратите внимание, что это свойство POJO, а не имя столбца БД), тогда этот код нужно будет изменить. Однако я бы сказал, что это очень редкая ситуация, за исключением самых новых новых проектов, в которых схемы базы данных все еще находятся в движении, и после того, как схемы базы данных установлены, этот недостаток редко вызывает проблемы.

Другой совет: после того, как вы нажмете определенное количество параметров, передаваемых в (например, 4-5), создайте объект параметра , чтобы обернуть ваши параметры в один прекрасный объект, который можно передать.

0 голосов
/ 27 апреля 2018

Существует не очень хорошее решение для этого. Если бы я был тобой, я бы рефакторинг это как:

@GetMapping
public ResponseEntity<List<Book>> searchBooksByTitleAndAuthor(
        @RequestParam(value = "title", required = false) String title,
        @RequestParam(value = "author", required = false) String author) {
    List<Book> books;
    if (StringUtils.isBlank(title) && StringUtils.isBlank(author)) {
        log.info("Empty request");
        return new ResponseEntity<>(HttpStatus.BAD_REQUEST);
    } else if (!StringUtils.isBlank(title) && !StringUtils.isBlank(author)) {
        books = bookService.getBooksByTitleAndAuthor(title, author);
    } else if (StringUtils.isBlank(author)) {
        books = bookService.getBooksByTitle(title);
    } else {
        books = bookService.getBooksByAuthor(author);
    }
    if (books.isEmpty()) {
        log.info("No books found with title = " + title + " and author = " + author);
        return new ResponseEntity<>(HttpStatus.NO_CONTENT);
    }
    return new ResponseEntity<>(books, HttpStatus.OK);
}

С этим решением вы обработаете один раз ответ.


Один совет

Если не отправлено ни одного параметра, вернуть неверный запрос о том, что для этой службы требуется хотя бы один параметр.

0 голосов
/ 27 апреля 2018

Я бы написал так:

@GetMapping
public List<Book> searchBooksByTitleAndAuthor(@RequestParam String title, @RequestParam String author) {
    return bookService.getBooksByTitleAndAuthor(title, author);
}

Я бы оставил ResponseEntity создание и HttpStatus управление Spring. Что касается null или управления пустыми значениями, я бы оставил это для службы или запроса к базе данных, который следует.

Кроме того, параметры, которые вы записали в аннотации RequestParam, являются значениями по умолчанию и могут быть упрощены.

Наконец, зачем вам регистрировать все запросы? В чем смысл? Если вы стремитесь к производству, ваш информационный журнал будет рассылаться по многочисленным запросам, а деловая информация в любом случае не будет принадлежать техническим журналам.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...