리팩터링 2판 - Java로 정리 - (3) statement() 함수 쪼개기

https://www.yes24.com/Product/Goods/89649360

리팩터링 2판 - 예스24

1. 각각의 부분으로 나눌 수 있는 지점 찾기

statement() 메서드처럼 긴 함수를 리팩터링할 때는 먼저 전체 동작을 각각의 부분으로 나눌 수 있는 지점을 찾아야 합니다. 중간 지점의 switch문이 눈에 띕니다.

package org.study.refactoringpractice.play;

import java.text.NumberFormat;
import java.util.List;
import java.util.Locale;
import java.util.Map;

public class Theater {

    public String statement(Invoice invoice, Map<String, Play> plays) {
        int totalAmount = 0;
        int volumeCredits = 0;
        String result = String.format("statement for (customer: %s)\n", invoice.getCustomerName());
        NumberFormat currencyFormatter = NumberFormat.getCurrencyInstance(Locale.US);

        for (Performance performance : invoice.getPerformances()) {
            Play play = plays.get(performance.getPlayID());
            int thisAmount = 0;

			// 여기!!!
            switch (play.getType()) {
                case "tragedy": // 비극
                    thisAmount = 40_000;
                    if (performance.getAudience() > 30) {
                        thisAmount += 1000 * (performance.getAudience() - 30);
                    }
                    break;
                case "comedy": // 희극
                    thisAmount = 30_000;
                    if (performance.getAudience() > 20) {
                        thisAmount += 10_000 + 500 * (performance.getAudience() - 20);
                    }
                    thisAmount += 300 * performance.getAudience();
                    break;
                default:
                    throw new IllegalArgumentException(String.format("Unknown genre: %s", play.getType()));
            }
            // 포인트를 적립한다.
            volumeCredits += Math.max(performance.getAudience() - 30,  0);
            // 희극 관객 5명마다 추가 포인트를 제공한다.
            if ("comedy".equals(play.getType())) {
                volumeCredits += Math.floor(performance.getAudience() / 5);
            }

            // 청구 내역을 출력한다.
            ;
            result += String.format("%s: %s (%d seats)\n", play.getName(), currencyFormatter.format(thisAmount / 100), performance.getAudience());
            totalAmount += thisAmount;
        }
        result += String.format("Total amount: %s\n", currencyFormatter.format(totalAmount / 100));
        result += String.format("You earned: %s points\n", volumeCredits);
        return result;

    }
}

해당 switch문은 한 번의 공연에 대한 요금을 계산하고 있습니다.

switch 문을 별도의 함수로 추출해서 방금 파악한 내용을 코드에 반영할 것입니다.

추출한 함수의 이름은 매우 중요합니다. 

함수의 이름은 그 코드가 하는 일을 잘 설명해야 합니다.

2. 함수 추출하기 - amountFor()

package org.study.refactoringpractice.play;

import java.text.NumberFormat;
import java.util.List;
import java.util.Locale;
import java.util.Map;

public class Theater {

    public String statement(Invoice invoice, Map<String, Play> plays) {
        int totalAmount = 0;
        int volumeCredits = 0;
        String result = String.format("statement for (customer: %s)\n", invoice.getCustomerName());
        NumberFormat currencyFormatter = NumberFormat.getCurrencyInstance(Locale.US);

        for (Performance performance : invoice.getPerformances()) {
            Play play = plays.get(performance.getPlayID());
            
            int thisAmount = amountFor(performance, play);    <--- 바뀐 지점
            
            // 포인트를 적립한다.
            volumeCredits += Math.max(performance.getAudience() - 30,  0);
            // 희극 관객 5명마다 추가 포인트를 제공한다.
            if ("comedy".equals(play.getType())) {
                volumeCredits += Math.floor(performance.getAudience() / 5);
            }

            // 청구 내역을 출력한다.
            ;
            result += String.format("%s: %s (%d seats)\n", play.getName(), currencyFormatter.format(thisAmount / 100), performance.getAudience());
            totalAmount += thisAmount;
        }
        result += String.format("Total amount: %s\n", currencyFormatter.format(totalAmount / 100));
        result += String.format("You earned: %s points\n", volumeCredits);
        return result;

    }
    
    // 한 번의 공연에 대한 요금을 계산하는 메서드
    private int amountFor(Performance performance, Play play) {        <--- 값이 바뀌지 않는 변수는 매개변수로 전달
        int thisAmount = 0;
        switch (play.getType()) {
            case "tragedy": // 비극
                thisAmount = 40_000;
                if (performance.getAudience() > 30) {
                    thisAmount += 1000 * (performance.getAudience() - 30);
                }
                break;
            case "comedy": // 희극
                thisAmount = 30_000;
                if (performance.getAudience() > 20) {
                    thisAmount += 10_000 + 500 * (performance.getAudience() - 20);
                }
                thisAmount += 300 * performance.getAudience();
                break;
            default:
                throw new IllegalArgumentException(String.format("Unknown genre: %s", play.getType()));
        }
        return thisAmount;
    }
}

2 - 2. 함수 추출하기를 할 때 주의사항

별도 함수로 추출할 때 새 함수에서 곧바로 사용할 수 없는 변수가 있는지 확인해야 합니다.

performance, play, thisAmount가 곧바로 사용할 수 없는 변수에 해당합니다.

performance와 play는 값을 변경하지 않기 때문에 매개변수로 전달하면 됩니다.

thisAmount는 함수 안에서 값이 바뀌므로, 이 값을 반환하도록 작성합니다.

3. 함수 추출하기 이후 좀 더 명확하게 표현하기 - 변수명 변경

함수를 추출하고 나면 추출된 함수를 자세히 들여다보면서 지금보다 더욱 명확하게 표현할 수 있는지 확인해야 합니다.

마틴 파울러는 함수의 반환값에는 항상 result라는 이름을 쓴다고 합니다. 그러면 그 변수의 역할을 쉽게 알 수 있다고 합니다. 나는 이 의견에 100% 옳은 방식이라고 동의하긴 어렵지만, 자신만의 방식을 세우고 그 방식을 고수하는 것은 합리적인 방식이라 생각합니다. 

// 한 번의 공연에 대한 요금을 계산하는 메서드
    private int amountFor(Performance aPerformance, Play play) {        <--- aPerformance 로 변경
        int result = 0;            <--- 함수의 반환값은 result로 변경
        switch (play.getType()) {
            case "tragedy": // 비극
                thisAmount = 40_000;
                if (performance.getAudience() > 30) {
                    thisAmount += 1000 * (performance.getAudience() - 30);
                }
                break;
            case "comedy": // 희극
                thisAmount = 30_000;
                if (performance.getAudience() > 20) {
                    thisAmount += 10_000 + 500 * (performance.getAudience() - 20);
                }
                thisAmount += 300 * performance.getAudience();
                break;
            default:
                throw new IllegalArgumentException(String.format("Unknown genre: %s", play.getType()));
        }
        return result;
    }

또한, 매개변수 performance 도 aPerformance로 변경합니다. 지금처럼 매개변수의 역할이 뚜렷하지 않을 때는 부정 관사(a/an)을 붙여줍니다. 이 방식은 켄트 벡의 [smalltalk Best Practice Patterns] 에서 차용한 방식입니다.

 

4. 마무리

오늘은 statement() 함수의 요금을 계산하는 로직을 amountFor() 함수로 추출하였고, 변수명도 알아보기 편하게 변경하는 작업을 수행했습니다. 리팩터링을 수행할 때마다 지난 게시글에 작성한 테스트 코드를 사용하여 코드가 정상적으로 동작하는지 수시로 체크해야 합니다. 테스트 코드를 만들었는데 사용하지 않으면 무용지물이 되겠죠? 아무리 간단한 수정이라도 리팩터링 후에는 항상 테스트하는 습관을 들이는 것이 바람직하다고 합니다. 조금씩 변경하고 매번 테스트하는 것은 리팩터링 절차의 핵심이라고 할 수 있겠습니다. 조금씩 수정하여 피드백 주기를 짧게 가져가는 습관을 기릅시다.