[내 코드가 그렇게 이상한가요?] 14장 리팩터링 : 기존의 코드를 성장시키는 기술

 

 

리팩터링이란?

 

 

실질적인 동작은 유지하면서, 구조만 정리하는 작업입니다.

 

 

실질적인 동작이 변하지 않았음을 확인할 수 있는 방법으로는 단위 테스트 등이 있습니다.

 

 

 

 

리팩터링 대상 코드는 아래와 같습니다.

class PurchasePointPayment{
    final CustomerId customerId; 		// 구매자의 ID
    final ComicId comicId;		 	// 구매할 웹툰의 ID
    final PurchasePoint consumptionPoint;	// 구매에 필요한 포인트
    final LocalDateTime paymentDateTime;	// 구매 일자

	PurchasePointPayment(final Customer customer, final Comic comic){
    
    	//1. 리팩터링 - 중첩 if문 제거
        //3. 리팩터링 - 조건을 읽기 쉽게 하기
    	if(customer.isEnabled()){
        
        	//2. 리팩터링 - 의미 단위로 로직 정리
        	customerId = customer.id;
            if(comic.isEnabled(){
            	comicId = comic.id;
                
                //4. 리팩터링 - 목적을 나타내는 메서드로 바꾸기
                if(comic.currentPurchasePoint.amount <=
                	customer.possessinPoint.amount){
                	consumptionPoint = comic.currentPurchasePoint;
                    paymentDateTime = LocalDateTime.now();
                }
                else {
                	throw new RuntimeException("보유하고 있는 포인트가 부족합니다.");
                }
            }
            else {
            	throw new IllegalArgumentException("현재 구매할 수 없는 만화입니다.");
            }
        }
        else {
        	throw new IllegalArgumentException("유효하지 않는 계정입니다.");
        }
    }
}

 

 

 

 

리팩터링 항목

 

1) 중첩 if문 정리

-> if문 조건을 반전

// 리팩터링 전
if(customer.isEnabled())

//리팩터링 후
if(!customer.isEnabled())

 

 

 

2) 의미 단위로 로직 정리하기

// 1) 조건 확인 로직을 모아둔다.

if(!customer.isEnabled(){...}
if(!comic.isEnabled(){...}
if(comic.currentPurchasePoint.amount <=
        customer.possessinPoint.amount){...}


// 2) 값 대입 로직을 모아둔다.
customerId = customer.id;
comicId = comic.id;
consumptionPoiont = comic.currentPurchasePoint;
paymentDateTime = LocalDateTime.now();

 

 

 

3) 조건을 읽기 쉽게 하기

-> 논리 부정 연산자 '!'를 사용하고 있으므로, 코드를 읽을 때 한 번 더 생각해서 '유효하지 않다'라고 바꿔 읽어야 합니다. 

-> isDisabled 메서드 추가 

//리팩터링 전
if(!customer.isEnabled())

//리팩터링 후
if(customer.isDisabled())

 

 

 

4) 목적을 나타내는 메서드로 바꾸기

// 리팩터링 전
if(comic.currentPurchasePoint.amount <=
        customer.possessinPoint.amount){...}
        
        
// 리팩터링 후
boolean isShortOfPoint(Comic comic){
	return customer.possessinPoint.amount < comic.currentPurchasePoint.amount;
}

if(customer.isShortOfPoint(comic){...}

 

 

 

 

 

 

단위 테스트로 리팩터링 중 실수 방지하기

1. 이상적인 구조의 클래스 기본 형태를 어느 정도 잡습니다.

 

2. 이 기본 형태를 기반으로 테스트 코드를 작성합니다.

 

3. 테스트를 실패시킵니다.

 

4. 테스트를 성공시키기 위한 최소한의 코드를 작성합니다.

 

5. 기본 형태의 클래스 내부에서 리팩터링 대상 코드를 호출합니다.

 

6. 테스트가 성공할 수 있도록, 조금씩 로직을 이상적인 구조로 리팩터링 합니다.

 

 

 

 

 

 

테스트가 없는 코드에 테스트를 추가해서 안전하게 리팩터링 할 수 있는 테크닉

 

 

문서화 테스트

 

문서화 테스트는 매서드의 사양을 분석하는 방법입니다.

 

 

 

아래와 같이 이름으로 의도를 확실하게 파악할 수 없는 코드가 있습니다.

public class MoneyManager{
    public static int calc(int v, boolean flag){
    	...
    }
}

 

 

 

일단 적당한 값을 입력해서 테스트를 작성합니다.

@Test
void characterizationTest(){
    int actual = MoneyManager.calc(1000, false);
    assertEquals(0, actual);
}

 

 

 

해당 코드는 실패하지만, 아래와 같은 어떤 값을 리턴하는지 알 수 있습니다.

org.opentest4j.AssertionFailedError:
Expected :0
Actual : 1000

 

 

그다음 테스트가 성공하도록 코드를 변경합니다.

@Test
void characterizationTest(){
    int actual = MoneyManager.calc(1000, false);
    assertEquals(1000, actual);
}

 

 

 

이렇게 테스트를 통해 매개변수에 따라 어떤 값이 나오는지 확인할 수 있습니다.

 

매개변수 v	매개변수 flag	리턴 값
1000	false	1000
2000	false	2000
3000	false	3000
1000	true	1100
2000	true	2200

 

• 매개변수 flag가 false라면, 매개변수 v를 그대로 리턴
• 매개변수 flag가 true라면, 매개변수 v를 활용해서 어떤 계산을 수행한 뒤 리턴

 

 

현재 calc가 MoneyManager 클래스의 메서드이므로, 대략 '매개변수 flag가 true일 때는 세율 10%를 포함한 금액을 계산하는 것이 아닐까?'라고 생각해 볼 수 있습니다.

 

 

 

 

 

리팩터링 시 주의 사항

1. 기능 추가와 리팩터링 동시에 하지 않기

리포지터리에 커밋할 때도 기능 추가와 리팩터링을 따로 구분해 두지 않으면, 이후에 해당 커밋이 기능 추가를 위한 커밋인지, 리팩터링을 위한 커밋인지 구분할 수 없습니다. 이렇게 되면 이후에 버그가 발생했을 때, 기능 추가로 버그가 발생한 것인지, 리팩터링으로 버그가 발생한 것인지 분석하기도 힘들어집니다.

 

 

 

2. 작은 단계로 실시하기

만약 리팩터링으로 메서드 이름 변경과 로직 이동을 했다면, 커밋을 따로따로 구분하는 것이 좋습니다. 둘을 모두 같은 커밋에 포함하면, 해당 커밋이 무슨 리팩터링을 실시한 것인지 구분하기 힘듭니다.

여러 번 커밋했다면, 풀 리퀘스트(Pull Request)를 작성하는 것이 좋습니다. 변경이 많으면 다른 사람이 변경한 코드와 충돌할 수있습니다.

 

 

 

3. 불필요한 사양은 제거 고려하기

이익에 거의 기여하지 않는 사양에 해당하는 코드는 일부러 개발 비용을 들여 리팩터링 하더라도, 개발 생산성 향상에 큰 도움이 되지 않습니다.