Рефакторинг Java, если еще

Question

У меня есть код if else, мне было интересно, есть ли более полезный / интеллектуальный способ его написания:

public void saveContent() throws Exception {
   if(book.isColored()) {
      book.setChoosen(“1234”);
   } else if (book.isAvailable()) {
      book.setChosen(“23498”);
   } else if (book.isAdults()) {
      book.setChosen(“0562”);
   } else {
      ReaderResponse response = reader.getReaderResponse();
      if (response != null) {
         book.setChosen(response.getName());
         }
      } else {
            book.setChosen(“4587”);
      }
   }
}

Метод возвращает void.

Answer 1

Просто хотел добавить версию Stream, но я не считаю ее достаточной только для нескольких случаев.

List<Pair<Predicate<Book>, String>> mapping;
mapping.add(Book::isColored, "1234");
mapping.add(Book::isAvailable, "23498");
mapping.add(Book::isAdults, "0562");
ReaderResponse response = reader.getReaderResponse();
mapping.add((anyBook) -> response == null, "4587");

String chosen = mapping.stream()
        .filter(p -> p.getKey().test(book))
        .map(Pair::getValue)
        .findFirst()
        .orElseGet(response::getName);

Или

mapping.add((anyBook) -> true, response.getName());
mapping.stream()
        .filter(p -> p.getKey().test(book))
        .map(Pair::getValue)
        .findFirst()
        .ifPresent(book::setChose);

Answer 2

Это не реализует ваш алгоритм точно (потому что есть некоторые неоднозначности), но демонстрирует использование enum в качестве шаблона стратегии. В этой форме намного проще добавить / настроить код, потому что все специальные функции каждого типа книг находятся в своем собственном местоположении (оно enum).

Обратите внимание, что это также помогает, когда книга может делиться на несколько типов (например, Coloured и Adult).

enum BookType {
    Coloured("1234"),
    Available("23498"),
    Adult("0562");

    private final String chosen;

    BookType(String chosen) {
        this.chosen = chosen;
    }

    public String getChosen() {
        return chosen;
    }

}

class Book {
    final Set<BookType> types = EnumSet.noneOf(BookType.class);
    Book book;

    public boolean hasType(BookType type) {
        return types.contains(type);
    }

    public void setChosen(String s) {

    }
}

public void saveContent() throws Exception {
    for(BookType type: BookType.values()) {
        if(book.hasType(type)) {
            book.setChosen(type.getChosen());
        }
    }
}

Answer 3

Я бы не сказал, что это более эффективный, более полезный или более умный способ сделать то же самое, но, конечно, я думаю, что код будет более чистым и тестируемым, если делегировать фактическое выбранное разрешение кода другому методу:

public void saveContent() throws Exception {
   String choosenCode = getChoosenCode(book);
   book.setChoosen(choosenCode);
}

static String getChoosenCode(Book book) throws Exception {
   if (book.isColored()) {
      return “1234”;
   }
   if (book.isAvailable()) {
      return “23498”;
   }
   if (book.isAdults()) {
      return “0562”;
   }
   ReaderResponse response = reader.getReaderResponse();
   return (response != null) 
      ? response.getName()
      : “4587”;
}

Как видите, нет необходимости в бесконечных блоках if-else из-за подхода раннего возврата. Это также обеспечивает возможность модульного тестирования выбранной части кода, что немного сложнее, если у вас есть void метод возврата.

Answer 4

Ну, это вопрос, о котором я думаю месяцами. По моему мнению, нелегко написать этот код таким образом, чтобы он был разумным и пригодным для начинающих программистов. Хорошим способом было бы использование оператора switch, но его нельзя использовать для логических операторов.

Я бы использовал код, упомянутый в вопросах, но я бы переместил его в один класс / контроллер / службу для более сложного кода (это было бы лучше для тестирования сложных поведений).

Answer 5

Вы можете реализовать шаблон проектирования стратегии в качестве альтернативы конструкции if-else

Answer 6

Введение локальной переменной в середине этого вызывает проблемы. Одним из способов решения этой проблемы является введение другого метода - не бойтесь маленьких методов.

public void saveContent() throws Exception {
    book.setChoosen(
        book.isColored()   ? “1234"  :
        book.isAvailable() ? “23498” :
        book.isAdults()    ? “0562”  :
        readerResponse()
    );
}
private String readerResponse() throws Exception {
    ReaderResponse response = reader.getReaderResponse();
    return response == null ? “4587” : response.getName();
}

? : - это условный оператор, часто называемый троичным оператором.

В случае, если getReaderResponse не имеет побочных эффектов, вы можете повторить вызов. get методы обычно не имеют побочных эффектов, но я чувствую, что это вполне может быть. Я не уверен, куда брошен Exception - я предполагаю, что он предназначен для замены на подтип.

Answer 7

Оператор Switch не поможет, потому что вы проверяете различные типы атрибутов. Я бы использовал if statements, но вместо "магических чисел" я бы использовал static final константы. И я бы удалил внутри последнего оператора else отдельный метод.