Spring Boot 리팩토링

4번째 필수 강의 Part 1. ch 6 요약

리팩토링

리팩토링이란 결과의 변경 없이 코드의 구조를 재조정한다는 뜻이다(위키백과). 주로 코드의 가독성을 높이고 유지보수를 편하게 하는 것을 목적으로 한다. 코드를 더 개선할 여지를 남기지 않고 처음부터 잘 짤 수 있다면 좋겠지만, 실제 현업에서는 서비스 오픈 후 정책 변경, 기능 추가로 인해 프로젝트의 복잡성과 규모가 확대될 수밖에 없다고 한다. 리팩토링을 통해 그 복잡성을 줄여줄 수 있는 것이다.

리팩토링은 주로

• 일정에 맞추느라 코드 품질이 다소 떨어졌던 부분,
• 사용자 경험에 큰 영향을 끼치는 부분,
• 추후 변경 가능성이 높은 부분,
• 타 서비스에서도 활용할 만한 공통적인 기능 (오픈소스로 API 개방 등)
• 테스트코드
• 그 외 새로운 기술로 성능/유지보수성 향상이 가능한 부분

등에 초점을 맞춰 진행한다.

---

강의를 따라 만들었던 DMaker을 리팩토링해본다.

1. final

먼저 DMakerController에서 메서드들이 받는 파라미터는 변경되지 않아야 하므로 final을 붙여줄 수 있다.

2. @Transactional

다음으로, DMakerService에서 get인 메서드들에도 추후에 기능이 추가될 수도 있으므로 데이터가 변경되는 일을 방지하기 위해 @Transactional 애너테이션을 추가해준다.

3. @NonNull

DMakerService에서 validateCreateDeveloperRequest처럼 받는 파라미터들 중 null이면 안되는 것들에 @NonNull을 붙여줄 수 있다. 애너테이션을 추가함으로써 if문을 써서 null을 받으면 따로 처리할 코드를 적어줄 필요가 없어진다.

4. createDeveloperFromRequest

createDeveloper에서 developer 객체를 build하는 것도 큰 로직은 아니지만 길어서 줄여줄 방법을 찾을 수 있다. 방법은 여러가지이고, 정답이라고 콕 짚어 말할 수 있는 건 없다! Developer 엔티티에 따로 넣어줄 수도 있다.

하지만 Entity는 DB와의 연동을 위한 클래스이고, 무엇보다 이 createDeveloper 메서드 하나만을 위해 메서드를 만드는 것은 바람직한 방향은 아니다. 하여 메서드의 위치를 Developer 엔티티에서 Service로 옮겨주고, 접근제어자와 이름도 바꿔주었다.

	private Developer createDeveloperFromRequest(CreateDeveloper.Request request){
        return Developer.builder()
                .developerLevel(request.getDeveloperLevel())
                .developerSkillType(request.getDeveloperSkillType())
                .experienceYears(request.getExperienceYears())
                .memberId(request.getMemberId())
                .name(request.getName())
                .age(request.getAge())
                .statusCode(StatusCode.EMPLOYED)
                .build();
    }

취향 차이긴 하지만, 1회성 변수인 경우에는 따로 변수를 선언하지 않고 매개변수로 넣어주는 것도 하나의 방법이다.

	@Transactional
    public CreateDeveloper.Response createDeveloper(CreateDeveloper.Request request) {
        validateCreateDeveloperRequest(request);

        return CreateDeveloper.Response.fromEntity(
                developerRepository.save(
                        createDeveloperFromRequest(request))
        );
    }

리팩토링을 할 때, test를 수시로 같이 돌려주는 편이 좋다. fromEntity가 developer를 받아야 하는데, 여기서 save에서 값을 반환하는 부분을 Mocking하지 않아서 NullPointerException이 발생해 테스트가 실패했다.
우선 fromEntity에서 받는 파라미터에도 @NonNull을 붙여주고, createDeveloper를 모킹한 것을 테스트에 추가해주면 성공하는 것을 확인할 수 있다.

@Test
    public void testCreateDeveloperSuccess(){
        // given
        given(developerRepository.findByMemberId(anyString()))
                .willReturn(Optional.empty());
        given(developerRepository.save(any()))
                .willReturn(defaultDeveloper);
        ArgumentCaptor<Developer> captor =
                ArgumentCaptor.forClass(Developer.class);
                ...

5. 중복된 예외처리 빼내기

또, getDeveloperDetail과 editDeveloper는 memberId의 developer가 항상 있어야 하는 경우이다. 두 메서드에 중복되는 코드가 있으므로 리팩토링 해 줄 수 있다.

	private Developer getDeveloperByMemberId(String memberId){
        return developerRepository.findByMemberId(memberId)
                .orElseThrow(() ->
                        new DMakerException(DMakerErrorCode.NO_DEVELOPER)
                );
    }

이렇게 빼준 다음, getDeveloperDetail과 editDeveloper는 다음과 같이 수정할 수 있다.

	@Transactional
    public DeveloperDetailDTO getDeveloperDetail(String memberId) {
        return DeveloperDetailDTO.fromEntity(getDeveloperByMemberId(memberId));
    }

    @Transactional
    public DeveloperDetailDTO editDeveloper(String memberId, EditDeveloper.Request request) {
        validateDeveloperLevel(request.getDeveloperLevel(), request.getExperienceYears());

        Developer developer = getDeveloperByMemberId(memberId);

        developer.setDeveloperLevel(request.getDeveloperLevel());
        developer.setDeveloperSkillType(request.getDeveloperSkillType());
        developer.setExperienceYears(request.getExperienceYears());
        return DeveloperDetailDTO.fromEntity(developer);
    }

이 때 developer 객체에 수정할 내용을 set해주는 코드 역시 응집력이 있어 별개의 메서드로 빼줄 수 있다.

	private Developer getUpdatedDeveloperFromRequest(EditDeveloper.Request request, Developer developer) {
        developer.setDeveloperLevel(request.getDeveloperLevel());
        developer.setDeveloperSkillType(request.getDeveloperSkillType());
        developer.setExperienceYears(request.getExperienceYears());
        return developer;
    }

일회성 변수들을 제거한 editDeveloper는 다음과 같다.

    @Transactional
    public DeveloperDetailDTO editDeveloper(String memberId, EditDeveloper.Request request) {
        validateDeveloperLevel(request.getDeveloperLevel(), request.getExperienceYears());
        return DeveloperDetailDTO.fromEntity(
                getUpdatedDeveloperFromRequest(request,
                        getDeveloperByMemberId(memberId)));
    }

---

6. Magic Number (1)

필요없는 파라미터나 중복된 validation은 지워주고, validation 메서드 안에 하드코딩된 숫자를 상수화 시키거나, ENUM에 같이 담아서 처리할 수 있다.

3가지 방법이 있는데, 먼저 상수로 빼내는 방법이 있다. constant라는 패키지를 만들어주고, DMakerConstant 클래스를 만들어서 안에 시니어 개발자의 최소 경력, 주니어 개발자의 최소경력을 상수로 넣어준다.

public class DMakerConstant {
    public static final Integer MIN_SENIOR_EXPERIENCE_YEARS = 10;
    public static final Integer MAX_JUNIOR_EXPERIENCE_YEARS = 4;
}

이 상수를 사용하면 기존의 validation 메서드는 다음과 같이 수정할 수 있다.

    private void validateDeveloperLevel(DeveloperLevel developerLevel, Integer experienceYears) {
        // business validation
        if ((developerLevel == DeveloperLevel.SENIOR)
                && (experienceYears <MIN_SENIOR_EXPERIENCE_YEARS)) {
//            throw new RuntimeException("SENIOR needs 10 years of experience.");
            throw new DMakerException(DMakerErrorCode.LEVEL_EXPERIENCE_YEARS_NOT_MATCHED);
        }
        if ((developerLevel == DeveloperLevel.JUNGNIOR) 
                && (experienceYears < MAX_JUNIOR_EXPERIENCE_YEARS 
                || experienceYears > MIN_SENIOR_EXPERIENCE_YEARS)) {
            throw new DMakerException(DMakerErrorCode.LEVEL_EXPERIENCE_YEARS_NOT_MATCHED);
        }
        if ((developerLevel == DeveloperLevel.JUNIOR) 
                && (experienceYears > MAX_JUNIOR_EXPERIENCE_YEARS)) {
            throw new DMakerException(DMakerErrorCode.LEVEL_EXPERIENCE_YEARS_NOT_MATCHED);
        }

여기에서는 사용하는 두 곳이 전부 이 파일에 모여있어서 큰 문제가 되지 않지만, 프로젝트 규모가 커지고 여러 군데에 흩어져있는 경우 골치아파질 수 있다. 이렇게 바꾸어주면, 나중에 senior 연차를 수정하거나 할 때 한 곳만 수정해주면 된다. 따라서 이러한 특정 값은 무조건 상수를 설정해 넣어주는 편이 좋다.

• level과 경력이 일치하지 않는 경우 예외가 잘 던져지는지 테스트를 만들어 확인해볼 수 있다. 다만 기존에 만들어둔 defaultCreateDeveloper가 예외가 발생하지 않는 객체이므로 메서드화하여 예외가 발생하도록 만든다.

    private CreateDeveloper.Request getCreateRequest(
            DeveloperLevel developerLevel,
            DeveloperSkillType developerSkillType,
            Integer experienceYears) {

        return CreateDeveloper.Request.builder()
                .developerLevel(developerLevel)
                .developerSkillType(developerSkillType)
                .experienceYears(experienceYears)
                .memberId("memberId")
                .name("name")
                .age(40)
                .build();
    }

given은 필요없는데 넣은 채로 테스트를 진행하면 사용하지 않는 mocking이 존재한다고 테스트가 실패한다. 지우고 진행하면 통과한다.

    @Test
    public void testCreateDeveloperSeniorYearMismatch(){
        // given
        // when
        DMakerException dMakerException = assertThrows(DMakerException.class,
                () -> dMakerService.createDeveloper(
                        getCreateRequest(SENIOR, FRONT_END, MIN_SENIOR_EXPERIENCE_YEARS-1))
        );

        assertEquals(DMakerErrorCode.LEVEL_EXPERIENCE_YEARS_NOT_MATCHED,
                dMakerException.getDMakerErrorCode());
    }

7. Magic Number (2)

상수로 설정하는 것도 좋지만, 이름이 좀 길고 중복된다는 느낌을 받을 수도 있어 enum을 사용할 수도 있다.

@AllArgsConstructor
@Getter
public enum DeveloperLevel {
    NEW("신입 개발자", 0, 0),
    JUNIOR("주니어 개발자", 1, MAX_JUNIOR_EXPERIENCE_YEARS),
    JUNGNIOR("중간 개발자",
            MAX_JUNIOR_EXPERIENCE_YEARS+1,
            MIN_SENIOR_EXPERIENCE_YEARS-1),
    SENIOR("시니어 개발자", MIN_SENIOR_EXPERIENCE_YEARS, 70);

    private final String description;
    private final Integer minExperienceYears;
    private final Integer maxExperienceYears;
}

enum을 설정해둔 후 검증 메서드를 단순하게 수정해줄 수 있다.

    private void validateDeveloperLevel(
            DeveloperLevel developerLevel, Integer experienceYears) {
        if (experienceYears < developerLevel.getMinExperienceYears() ||
        experienceYears > developerLevel.getMaxExperienceYears()) {
            throw new DMakerException(DMakerErrorCode.LEVEL_EXPERIENCE_YEARS_NOT_MATCHED)
        }
    }

테스트를 실행해보면 잘 통과하는 것을 확인할 수 있다.

8. Magic Number (3)

일반적인 케이스에서 사용하기는 힘들지만 이 경우에는 사용할 수 있는 방법으로, 아예 enum에 function을 넘겨버릴 수도 있다. Function은 함수형 프로그래밍을 지원해주기 위한 인터페이스로, 어떤 타입의 값을 받아서 또 어떤 타입의 값을 반환해준다.

@AllArgsConstructor
@Getter
public enum DeveloperLevel {
    NEW("신입 개발자", years -> years == 0),
    JUNIOR("주니어 개발자", years -> years <= MAX_JUNIOR_EXPERIENCE_YEARS),
    JUNGNIOR("중간 개발자", years -> years > MAX_JUNIOR_EXPERIENCE_YEARS &&
            years < MIN_SENIOR_EXPERIENCE_YEARS),
    SENIOR("시니어 개발자", years -> years >= MIN_SENIOR_EXPERIENCE_YEARS);

    private final String description;
    private final Function<Integer, Boolean> validateFunction;

    public void validateExperienceYears(Integer years){
        if (!validateFunction.apply(years))
            throw new DMakerException(DMakerErrorCode.LEVEL_EXPERIENCE_YEARS_NOT_MATCHED);
    }
}

• Integer를 받고 Boolean을 반환하는 함수를 만든다.
• Function이 private이기 때문에 외부에 값을 돌려주기 위한 public 메서드를 하나 만든다.
• function의 결과가 false면 예외를 던지게 만들었다.

enum을 이렇게 수정하면, DMakerService의 validateDeveloperLevel을 삭제하고, validateDeveloperLevel을 호출하는 나머지 두 곳도 훨씬 간결하게 수정해줄 수 있다.

    private void validateCreateDeveloperRequest(@NonNull CreateDeveloper.Request request) {
        request.getDeveloperLevel().validateExperienceYears(
                request.getExperienceYears());
        developerRepository.findByMemberId(request.getMemberId())
                .ifPresent(developer -> {
                    throw new DMakerException(DMakerErrorCode.DUPLICATED_MEMBER_ID);
                });
    }

    @Transactional
    public DeveloperDetailDTO editDeveloper(String memberId, EditDeveloper.Request request) {
        request.getDeveloperLevel().validateExperienceYears(request.getExperienceYears());
        return DeveloperDetailDTO.fromEntity(
                getUpdatedDeveloperFromRequest(request,
                        getDeveloperByMemberId(memberId)));
    }

이렇게 고쳐준 후 테스트를 돌려보면, 실패 없이 통과하는 것을 확인할 수 있다.

• 팀의 코드 컨벤션도 중요하므로, 줄바꿈 등도 맞춰 주며 이를 커밋할 때는 polishing이라고 메시지를 남겨준다.

---

리팩토링은 끝이 없기 때문에, 클린코드에 대해 더 공부해보거나, 구조적인 리팩토링에 관심이 있다면 객체지향 및 TDD에 대한 서적을 찾아보는 등 추가적으로 노력하여 개발자로서의 역량을 늘릴 수 있을 것이다.