[Refactoring] 긴 매개변수 목록 (Long Parameter List)

백기선 님의 리팩터링 강의를 들으며 요약한 내용입니다.

 

4. 긴 매개변수 목록

어떤 함수에 매개변수가 많을수록 함수의 역할을 이해하기 어려워 진다.

• 과연 그 함수는 한가지 일만 하고있는것이 맞는가?
• 불필요한 매개변수는 없는가?
• 하나의 레코드로 뭉칠수 있는 매개변수 목록은 없는가?

 

어떤 매개변수를 다른 매개변수를 통해 알아낼 수 있다면

=> “매개변수를 질의 함수로 바꾸기 (Replace Parameter with Query)”를 사용할 수 있다.

 

기존 자료구조에서 세부적인 데이터를 가져와서 여러 매개변수로 넘기는 대신, “객체 통째로 넘기기 (Preserve Whole Object)”를 사용할 수 있다.

일부 매개변수들이 대부분 같이 넘겨진다면, “매개변수 객체 만들기 (Introduce Parameter Object)”를 적용할 수 있다.

 

매개변수가 플래그로 사용된다면, “플래그 인수 제거하기 (Remove Falg Argument)”를 사용할 수 있다.

 

여러 함수가 일부 매개변수를 공통적으로 사용한다면 “여러 함수를 클래스로 묶기 (Combine Functions into Class)”를 통해 매개변수를 해당 클래스의 필드로 만들고, 메서드에 전달할 매개변수의 수를 줄일 수 있다.

 

1. 매개변수를 질의 함수로 바꾸기 (Replace Parameter with Query)

public class Order {

    private int quantity;

    private double itemPrice;

    public Order(int quantity, double itemPrice) {
        this.quantity = quantity;
        this.itemPrice = itemPrice;
    }

    public double finalPrice() {
        double basePrice = this.quantity * this.itemPrice;
        int discountLevel = this.quantity > 100 ? 2 : 1;
        return this.discountedPrice(basePrice, discountLevel);
    }

    private double discountedPrice(double basePrice, int discountLevel) {
        return discountLevel == 2 ? basePrice * 0.9 : basePrice * 0.95;
    }
}

위 Order 라는 class는 finalPrice() 안에서 discountLevel을 계산한다.

이후 discountPrice(double basePrice, int discountLevel) 에 인자로 절달하고 있다.

 

즉, 함수를 호출하는 Order라는 Client가 책임지고 discountLevel를 생성하여 인자로 전달해주고 있다.

이를 다음과 같이 변경해 보자.

public double finalPrice() {
    double basePrice = this.quantity * this.itemPrice;
    return this.discountedPrice(basePrice);
}

private int discountLevel() {
    return this.quantity > 100 ? 2 : 1;
}

private double discountedPrice(double basePrice) {
    return discountLevel() == 2 ? basePrice * 0.90 : basePrice * 0.95;
}

변경 후 에는, discountPrice 메서드 스스로가 책임을 지게 된다.

스스로 해당 메서드를 호출하여 값을 얻어와야 한다. 파라미터 또한 1개로 줄어들었다!

 

2. 플래그 인수 제거하기 (Remove Falg Argument)

플래그를 사용하는 함수는 의미를 이해하기 어려우며, 각 플래그 간의 차이를 파악하기 어렵다.

예를 들어 다음 코드를 살펴보자. 어떠한 의미인지 한눈에 파악이 되는가?

bookConcert(customer, false), bookConcert(customer, true)

아마 해당 메서드 안까지 들어가서 flag의 의미를 파악해야 할 것 이다.

 

해당 flag 가 premium을 의미했다면, 다음과 같이 함수를 변경하여 의미 전달을 명확하게 하는 것이 더 좋다.

bookConcert(customer), premiumBookConcert(customer)

 

3. 여러 함수를 클래스로 묶기 (Combine Functions into Class)

우선 변경전의 코드는 다음과 같다. (일부 생략)

public class StudyDashboard {
    // 생략...

    private void print() throws IOException, InterruptedException {
        // 생략...

        latch.await();
        service.shutdown();

        try (FileWriter fileWriter = new FileWriter("participants.md");
             PrintWriter writer = new PrintWriter(fileWriter)) {
            participants.sort(Comparator.comparing(Participant::username));

            writer.print(header(totalNumberOfEvents, participants.size()));

            participants.forEach(p -> {
                long count = p.homework().values().stream()
                        .filter(v -> v == true)
                        .count();
                double rate = count * 100 / totalNumberOfEvents;

                String markdownForHomework = String.format("| %s %s | %.2f%% |\n", p.username(), checkMark(p, totalNumberOfEvents), rate);
                writer.print(markdownForHomework);
            });
        }
    }

    private String header(int totalEvents, int totalNumberOfParticipants) {
        StringBuilder header = new StringBuilder(String.format("| 참여자 (%d) |", totalNumberOfParticipants));

        for (int index = 1; index <= totalEvents; index++) {
            header.append(String.format(" %d주차 |", index));
        }
        header.append(" 참석율 |\n");

        header.append("| --- ".repeat(Math.max(0, totalEvents + 2)));
        header.append("|\n");

        return header.toString();
    }

    private String checkMark(Participant p, int totalEvents) {
        StringBuilder line = new StringBuilder();
        for (int i = 1 ; i <= totalEvents ; i++) {
            if(p.homework().containsKey(i) && p.homework().get(i)) {
                line.append("|:white_check_mark:");
            } else {
                line.append("|:x:");
            }
        }
        return line.toString();
    }
}

StudyDashboard의 print()는 header와 checkMark 메서드를 내부적으로 사용하고 있다.

이렇게 서로 연관된 메서드들을 하나의 class안으로 모두 이동시키면, 매서드 간에 서로 주고 받던 파라미터의 수 또한 줄어들 수 있다.

 

StudyPrinter라는 Class로 뽑아내면 다음과 같다.

관련된 메서드들을 모두 StudyPrinter 안으로 이동시켰다.

또한 3 메서드가 공통적으로 사용하던 totalNumberOfEvents 라는 변수또한 field에 만들어서 함수들의 파라미터 수를 줄일 수 있었다.

public class StudyPrinter {

    private int totalNumberOfEvents;
    private List<Participant> participants;

    public StudyPrinter(int totalNumberOfEvents, List<Participant> participants) {
        this.totalNumberOfEvents = totalNumberOfEvents;
        this.participants = participants;
    }

    public void print() throws IOException {
        try (FileWriter fileWriter = new FileWriter("participants.md");
             PrintWriter writer = new PrintWriter(fileWriter)) {
            this.participants.sort(Comparator.comparing(Participant::username));

            writer.print(header());

            this.participants.forEach(p -> {
                String markdownForHomework = getMarkdownForParticipant(p.username(), p.homework());
                writer.print(markdownForHomework);
            });
        }
    }

    private String getMarkdownForParticipant(String username, Map<Integer, Boolean> homework) {
        return String.format("| %s %s | %.2f%% |\n", username, checkMark(homework), getRate(homework));
    }

    double getRate(Map<Integer, Boolean> homework) {
        long count = homework.values().stream()
                .filter(v -> v == true)
                .count();
        return (double) (count * 100 / this.totalNumberOfEvents);
    }

    private String header() {
        StringBuilder header = new StringBuilder(String.format("| 참여자 (%d) |", this.participants.size()));

        for (int index = 1; index <= this.totalNumberOfEvents; index++) {
            header.append(String.format(" %d주차 |", index));
        }
        header.append(" 참석율 |\n");

        header.append("| --- ".repeat(Math.max(0, this.totalNumberOfEvents + 2)));
        header.append("|\n");

        return header.toString();
    }

    private String checkMark(Map<Integer, Boolean> homework) {
        StringBuilder line = new StringBuilder();
        for (int i = 1 ; i <= this.totalNumberOfEvents ; i++) {
            if(homework.containsKey(i) && homework.get(i)) {
                line.append("|:white_check_mark:");
            } else {
                line.append("|:x:");
            }
        }
        return line.toString();
    }
}

개인적 의견이지만, SRP가 떠오르는 리팩터링 방식 이었다.

해당 Printer Class가 변경되야 하는 이유는 단 한가지임에 충실한 과정인것 같다.

 

항상 관련된 상태와 행동을 한곳에 잘 정의한 응집도 높은 Class를 만들어야 할 것 이다.