Блог

Мне нужно проверить, разрешено ли пользователю отменять встречу. Прямо сейчас я сначала вызываю метод service, который возвращает логическое значение, и если оно равно false, то я вызываю другой метод, который возвращает причину в строке. Это работает, но я думаю, что это некрасиво. У вас есть какие-либо идеи, как я могу это улучшить? Или, может быть, этот подход подходит?

AppointmentService:

  public boolean isUserAllowedToCancelAppointment(int userId, int appointmentId) {
        User user = userService.findById(userId);
        Appointment appointment = findById(appointmentId);

        // only scheduled appointments can be canceled
        if(!appointment.getStatus().equals("scheduled")){
            return false;
        }
        // other conditions...
}


 public String getCancelNotAllowedReason(int userId, int appointmentId) {
        User user = userService.findById(userId);
        Appointment appointment = findById(appointmentId);

        if(!appointment.getStatus().equals("scheduled")){
            return "status";
        // other conditions and reasons...
}
  

Контроллер:

    @GetMapping("/{id}")
   public String showAppointmentDetail(@PathVariable("id") int appointmentId, Model model, @AuthenticationPrincipal CustomUserDetails currentUser) {

    if (appointmentService.isUserAllowedToCancelAppointment(currentUser.getId(), appointmentId)) {
        model.addAttribute("allowCancel", true);
    } else {
        model.addAttribute("allowCancel", false);
        model.addAttribute("cancelNotAllowedReason", appointmentService.getCancelNotAllowedReason(currentUser.getId(), appointmentId));
    }
   }
  

Создайте enum список возможных проблем:

 enum Issue {TOO_LATE, NOT_SCHEDULED, ... }
  

А затем создайте единый метод проверки, который возвращает Set<Issue> :

 public Set<Issue> validate(int userId, int appointmentId) { ... }
  

Затем вы можете проверить различные случаи и, если они применимы, добавить Issue в набор:

 Set<Issue> issues = EnumSet.noneOf(Issue.class);
if( not scheduled ) {
    issues.add(Issue.NOT_SCHEDULED);
}
if( too late ) {
    issues.add(Issue.TOO_LATE);
}
return issues;
  

Затем на сайте вызова вы можете проверить, issues.isEmpty() :

 Set<Issue> issues = appointmentService.validate(userId, appointmentId);
model.addAttribute("allowCancel", issues.isEmpty());
model.addAttribute("cancelNotAllowedReason", issues);
  

Вы выполняете два вызова, в то время как вы должны выполнить только один. Это действительно не рекомендуется.

Отбросьте логический метод и сохраните что-то вроде:

 public String getCancelNotAllowedReason(int userId, int appointmentId) {
        User user = userService.findById(userId);
        Appointment appointment = findById(appointmentId);

        if(!appointment.getStatus().equals("scheduled")){
            return "not yet scheduled";
        }
        if ( appointment.isDateTooClose()) {
           return "Too late to cancel";
        }
        // other conditions and reasons...
        return "Valid";
}
  

В вызывающем классе:

 if ( service.getCancellNotAllowedReason(1, 1).equals("Valid")) {
  //cancel
} else {
  // show information message
}
  

Однако, возможно, было бы лучше вернуть List<String> список причин. В противном случае ваш клиент может исправить первую проблему и предположить, что он может отменить ее, просто чтобы застрять на следующей проблеме.

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

   String reason = appointmentService.getCancelNotAllowedReason(currentUser.getId(), appointmentId);
  model.addAttribute("allowCancel", reason == null);
  model.addAttribute("cancelNotAllowedReason", reason);