깔끔한 코드를 추구하는 스프링 사용자라면 검토해라

• 코드에 중복된 부분은 없는가
• 코드가 무엇을 하는 것인지 이해하기 불편하지는 않은가.
• 코드가 자신이 있어야 할 자리에 있는가.
• 앞으로 변경이 일어난다면 어떤 것이 있을 수 있고, 그 변화에 쉽게 대응할 수 있게 작성되었는가.

upgradeLevels() 메서드 코드의 문제점

for 루프 속에 들어있는 if/elseif/else 블록들이 읽기 불편하다.

• 성격이 다른 여러 로직이 한데 섞여있다.
  • 레벨 변화 단계 + 업그레이드 조건 + 조건 충족 시 해야 하는 작업
  • 플래그를 두고 변경, 확인 후 업데이트…
• 확장성이 낮음
  • 레벨 개수만큼 조건문 반복
  • 새로운 레벨 추가 시
    • Level 이넘 수정
    • upgradeLevels() 내부에 레벨 업그레이드 로직 추가
  • 메서드 역할이 필드 변경 수준 이상이라면 점점 길고 복잡해지며 유지보수가 힘들어진다.
• 현재 레벨 + 업그레이드 조건 동시 비교
  • 성격이 다른 두 경우가 한 곳에서 처리될 수 있다.
    1. BASIC 이면서 로그인 횟수가 50이 안되는 경우
    2. 새로운 레벨이 추가되는 경우

Boolean changed = null;
if (**user.getLevel() == Level.BASIC** && **user.getLogin() >= 50**) {
  user.setLevel(Level.SILVER);
  changed = true;
} else if (user.getLevel() == Level.SILVER && user.getRecommend() >= 30) {
  user.setLevel(Level.GOLD);
  changed = true;
} else if (user.getLevel() == Level.GOLD) {
  changed = false;
} else {
  changed = false;
}
if (changed) { userDao.update(user); }

1. user.getLevel() == Level.BASIC
   • 현재 레벨 파악
2. user.getLogin() >= 50
   • 업그레이드 조건
3. user.setLevel(Level.SILVER);
   • 다음 단계의 레벨
   • 업그레이드를 위한 작업
4. if (changed) { userDao.update(user); }
   • changed: 해당 작업이 필요한지 알려주기 위한 임시 플래그

두 단계에 걸쳐서 조건을 비교하자.

• 첫 단계에서 레벨을 확인하고 각 레벨별로 다시 조건을 판단

upgradeLevels() 리팩토링

코드 기본 작업 흐름만 남겨두자.