Java가 중첩되어 있다면? 다른 접근법?

Question

그런 중첩 된 if else 문을 처리하는 다른 방법이 있습니까? 나는 너무 많은 else 문이 나쁘다는 것을 도처에서 읽었습니다. 이러한 경우를 처리하는 올바른 방법입니까 아니면 리팩토링 할 필요가 있습니까?

User user = loginHistoryService.findByUsername(loginRequest.getUsername()); 
    boolean isPasswordValid = bcryptPasswordEncoder.matches(loginRequest.getPassword(), user.getPassword()); 
    if (user.isDeleted()) { 
     throw new AccountDeletedAuthException("Account is Deleted"); 
    } else if (user.isLocked()) { 
     if (DateUtil 
       .addHours(user.getLockedTime(), Integer.parseInt(
         messageSource.getMessage(Constants.UNLOCK_ACCOUNT_AFTER_XX_HOURS, null, "24", null))) 
       .compareTo(DateUtil.getCurrentDateTime()) < 0 && isPasswordValid) { 
      logger.info("|*|*| Unlocking account. Account Lock Timer Over.. |*|*|*|"); 
      loginHistoryService.lockUserAccount(user.getUserId(), false); 
     } else { 
      throw new LockedException("Account is Locked"); 
     } 
    } else if (user.getUserStatusId() == UserStatus.INACTIVE.getStatusCode()) { 
     throw new InactiveAccountAuthException("Account is Inactive"); 
    } else if (user.getUserStatusId() == UserStatus.PENDING_VERIFICATION.getStatusCode()) { 
     throw new DisabledException("".trim() + user.getUserId()); 
    } else if (!isPasswordValid) { 

     List<Integer> loginAttemptStatuses = loginHistoryService.getLastThreeLoginAttempts(user.getUserId()); 
     loginHistoryService.createLoginHistoryEntry(user.getUserId(), LoginStatus.FAILURE.getStatusCode()); 
     int consecutiveFailedAttempts = 0; 
     for (int tempIndex = 0; tempIndex < loginAttemptStatuses.size(); tempIndex++) { 
      if (loginAttemptStatuses.get(tempIndex).intValue() == LoginStatus.FAILURE.getStatusCode()) { 
       consecutiveFailedAttempts++; 
      } else { 
       break; 
      } 
     } 
     if (consecutiveFailedAttempts == 3) { 
      loginHistoryService.lockUserAccount(user.getUserId(), true); 
     } 
     throw new InvalidPasswordAuthException("".trim() + consecutiveFailedAttempts); 
    } 

Answer 1

경우에 따라 비즈니스 로직을 처리 할 때 이와 같은 구조를 갖게 될 수도 있습니다. switch 문을 만들어 약간 정리할 수 있지만,이 경우 많은 도움이되지는 않습니다.

대신이 문제를 해결하기 위해 노력하고, 나는 확실히 추상화의 많은 층 사이를 이동하지 않는이 거대한 덩어리를 리팩토링을 시도한다. 이 블록 여기 예로

테이크 : 사용자가 잠겨있는 경우 블록의 시작 부분에서

else if (user.isLocked()) { 
    if (DateUtil 
      .addHours(user.getLockedTime(), Integer.parseInt(
        messageSource.getMessage(Constants.UNLOCK_ACCOUNT_AFTER_XX_HOURS, null, "24", null))) 
      .compareTo(DateUtil.getCurrentDateTime()) < 0 && isPasswordValid) { 
     logger.info("|*|*| Unlocking account. Account Lock Timer Over.. |*|*|*|"); 
     loginHistoryService.lockUserAccount(user.getUserId(), false); 
} 

, 우리는 묻는 "user level abstraction layer"에 노력하고 있습니다. 결국, 우리는 또한 동일한 추상화 수준에 있으며, 사용자의 잠금을 해제합니다. 하지만 우리는 몇 가지의 int를 구문 분석에 가서 사이에하는 messagesource에서 데이터를 끌어와 같은 다른 경건 치 아니한 물건을 일부 날짜 연산을 수행 할.

는 이러한 것들을 꺼내보십시오. 이 읽기 방법에 코드의 상단 블록을 비교 :

else if(user.isLocked() && canUnlockUser(user)) { 
    logger.info("|*|*| Unlocking account. Account Lock Timer Over.. |*|*|*|"); 
    loginHistoryService.lockUserAccount(user.getUserId(), false); 
} 

또는 코드에 해당 남아

else if(user.isLocked()) { 
    if(canUnlockUser(user)){ 
     logger.info("|*|*| Unlocking account. Account Lock Timer Over.. |*|*|*|"); 
     loginHistoryService.lockUserAccount(user.getUserId(), false); 
    } else { 
     throw new LockedException("Account is Locked"); 
    }  
} 

니스 (감사를 지적 tobias_k합니다)! 실제로 첫 번째 모습에서 무슨 일이 벌어지고 있는지 말할 수 있습니다. 당신은이 모든 것을 혼란에 적용 할 수 있습니다.

내가 언급 한 블록의 가독성을 높이는 또 다른 사항은 다음과 같습니다. loginHistoryService.lockUserAccount() 및 loginHistoryService.unlockUserAccount() 방법을 모두 고려하십시오. 이와 같은 부 울린 플래그를 전달하는 것은 읽을 수없고 추악하며 버그를 도입하기 쉽습니다. 그래서, 결국, 블록은

else if(user.isLocked() && canUnlockUser(user)) { 
    logger.info("|*|*| Unlocking account. Account Lock Timer Over.. |*|*|*|"); 
    loginHistoryService.unlockUserAccount(user.getUserId()); 
} 

또는

else if(user.isLocked()) { 
    if(canUnlockUser(user)){ 
     logger.info("|*|*| Unlocking account. Account Lock Timer Over.. |*|*|*|"); 
     loginHistoryService.lockUserAccount(user.getUserId(), false); 
    } else { 
     throw new LockedException("Account is Locked"); 
    }  
} 

Answer 2

당신의 코드가 복잡하지 읽기 것이라고하지만 읽을 약간 복잡하다. 당신의 자리에서, 나는 약간의 포맷팅을하는 자바 코드를 작업 할 것이다. 그런 다음 각 큰 하위 조건부 블록 코드를 개인 메서드로 이동합니다. 많은 "포함 된"포함 된 읽기 및 유지 관리 어려울 수 있습니다. 관련 이름을 사용하여 개인 방법으로 분할하는 것이 편리 할 수 ​​있습니다. 귀하의 경우에는

, 나는 그것을는 사실을 알게 될 것입니다. 그것은 그렇게 볼 수 있었다 : 아무 것도이 후 다음없는 경우

가

User user = loginHistoryService.findByUsername(loginRequest.getUsername()); 
boolean isPasswordValid = bcryptPasswordEncoder.matches(loginRequest.getPassword(), user.getPassword()); 

if (user.isDeleted()) { 
    throw new AccountDeletedAuthException("Account is Deleted"); 
} 
else if (user.isLocked()) { 
    handleWhenUserLocked(user); 
} 
else if (user.getUserStatusId() == UserStatus.INACTIVE.getStatusCode()) { 
    throw new InactiveAccountAuthException("Account is Inactive"); 
} 
else if (user.getUserStatusId() == UserStatus.PENDING_VERIFICATION.getStatusCode()) { 
    throw new DisabledException("".trim() + user.getUserId()); 
} 
else if (!isPasswordValid) { 
    handleWhenPasswordNotValid(user); 
}