리팩터링 2판 - Java로 정리 - (4) play 변수 제거하기

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

리팩터링 2판 - 예스24

 

지난 게시글에서는 statement() 메서드 안에 요금을 계산하는 로직을 amountFor() 메서드로 추출하였고,

변수명도 명확하게 변경했습니다.

하지만 아직 고쳐야할 부분이 남아 있는데요.

play 매개 변수가 눈에 거슬립니다.

 

1. play 변수 제거하기 

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 aPerformance, 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;
    }
}

play 매개변수는 개별 공연(aPerformance)에서 얻기 때문에 애초에 매개변수로 전달할 필요가 없습니다. 

이렇게 쓸데 없이 전달하는 매개변수는 제거하는 것이 좋겠죠?

1 - 2. 임시 변수를 질의 함수로 바꾸기

package org.study.refactoringpractice.play;

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

public class Theater {

	// 이게 질의 함수!
    private Play playFor(List<Play> plays, Performance aPerformance) {
        return plays[aPerformance.playID];
    }
    
    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 = playFor(plays, performance);   <---- 바뀐 지점!
            
            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;
    }
    
    ...
    
}

바뀐 지점을 보면 play 지역 변수에 playFor() 이라는 질의 함수를 호출하도록 수정했습니다.

컴파일-테스트-커밋을 수행해서 문제가 없는지 살펴봅니다.

1 - 3. 변수 인라인하기

package org.study.refactoringpractice.play;

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

public class Theater {

	// 이게 질의 함수!
    private Play playFor(List<Play> plays, Performance aPerformance) {
        return plays[aPerformance.playID];
    }
    
    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()) {
            
            int thisAmount = amountFor(performance, playFor(plays, performance));    <---- 바뀐 지점!
            
            volumeCredits += Math.max(performance.getAudience() - 30,  0);
       
            if ("comedy".equals(playFor(plays, performance).getType())) {         <--- 바뀐 지점 !
                volumeCredits += Math.floor(performance.getAudience() / 5);
            }

            result += String.format("%s: %s (%d seats)\n", playFor(plays, performance).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;
    }
    
    ...
    
}

amountFor()에 변수 인라인하기를 적용합니다.

역시 컴파일-테스트-커밋을 하여 문제가 없는지 체크!

2. plays 필드로 추출

Javascript와 달라 Java에서는 amountFor() 메서드와 playFor() 메서드에서 plays 에 접근하려면

매개변수로 전달해야 합니다.

play를 제거하는 목적이 매개변수를 제거하기 위한 것이었는데

plays를 매개변수에 추가하는 것은 앞뒤가 맞지 않습니다.

plays를 필드로  추출해서 클래스내의 모든 메서드가 접근할 수 있도록 하는 것이 좋겠습니다.

 

package org.study.refactoringpractice.play;

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

public class Theater {

    private Map<String, Play> plays;;   <-- 필드로 추출

    public Theater(Map<String, Play> plays) {
        this.plays = plays;
    }

    public String statement(Invoice invoice) {
        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()) {

            int thisAmount = amountFor(performance, playFor(performance));

            volumeCredits += Math.max(performance.getAudience() - 30, 0);
            
            if ("comedy".equals(playFor(performance).getType())) {
                volumeCredits += Math.floor(performance.getAudience() / 5);
            }

            result += String.format("%s: %s (%d seats)\n", playFor(performance).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 Play playFor(Performance aPerformance) {
        return plays.get(aPerformance.getPlayID());
    }

    private int amountFor(Performance aPerformance, Play play) {
        int result = 0;
        switch (playFor(aPerformance).getType()) {
            case "tragedy": // 비극
                result = 40_000;
                if (aPerformance.getAudience() > 30) {
                    result += 1000 * (aPerformance.getAudience() - 30);
                }
                break;
            case "comedy": // 희극
                result = 30_000;
                if (aPerformance.getAudience() > 20) {
                    result += 10_000 + 500 * (aPerformance.getAudience() - 20);
                }
                result += 300 * aPerformance.getAudience();
                break;
            default:
                throw new IllegalArgumentException(String.format("Unknown genre: %s", playFor(aPerformance).getType()));
        }
        return result;
    }


}

2 - 2. Theater에 생성하는 테스트코드와 PlayMain 코드 수정

package org.study.refactoringpractice.play;

import java.util.List;
import java.util.Map;

public class PlayMain {

    public static void main(String[] args) {

        Play hamlet = new Play("hamlet", "Hamlet", "tragedy");
        Play asYouLikeIt = new Play("as-like", "As You Like It", "comedy");
        Play othello = new Play("othello", "Othello", "tragedy");

        Performance hamletPerformance = new Performance("hamlet", 55);
        Performance asYouLikeItPerformance = new Performance("as-like", 35);
        Performance othelloPerformance = new Performance("othello", 40);

        Map<String, Play> plays = Map.of(
                "hamlet", hamlet,
                "as-like", asYouLikeIt,
                "othello", othello
        );

        Theater theater = new Theater(plays);

        Invoice invoice = new Invoice("BigCo",
                List.of(hamletPerformance, asYouLikeItPerformance, othelloPerformance));

        String result = theater.statement(invoice);
        System.out.println(result);
    }
}
------------------------------------------------------------------------------------------
package org.study.refactoringpractice.play;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;

import java.util.List;
import java.util.Map;

import static org.assertj.core.api.Assertions.assertThat;

class TheaterTest {

    private Theater theater;

    @BeforeEach
    void setUp() {
        Play hamlet = new Play("hamlet", "Hamlet", "tragedy");
        Play asYouLikeIt = new Play("as-like", "As You Like It", "comedy");
        Play othello = new Play("othello", "Othello", "tragedy");
        Map<String, Play> plays = Map.of(
                "hamlet", hamlet,
                "as-like", asYouLikeIt,
                "othello", othello
        );
        theater = new Theater(plays);
    }

    @DisplayName("원하는 문자열이 출력되는 자가진단 테스트")
    @Test
    void testOne() {
        // Given
        Performance hamletPerformance = new Performance("hamlet", 55);
        Performance asYouLikeItPerformance = new Performance("as-like", 35);
        Performance othelloPerformance = new Performance("othello", 40);

        String customerName = "BigCo";

        Invoice invoice = new Invoice(customerName,
                List.of(hamletPerformance, asYouLikeItPerformance, othelloPerformance));

        // When
        String result = theater.statement(invoice);
        String expected = "statement for (customer: BigCo)\n" +
                "Hamlet: $650.00 (55 seats)\n" +
                "As You Like It: $580.00 (35 seats)\n" +
                "Othello: $500.00 (40 seats)\n" +
                "Total amount: $1,730.00\n" +
                "You earned: 47 points\n";

        // Then
        assertThat(result).isEqualTo(expected);
    }
}

Theater를 생성할 때 plays를 초기화하도록 수정했습니다.

3. play 매개 변수 삭제

play 는 playFor(aPerformance)함수로 구할 수 있기 때문에

매개 변수에서 제거해줍시다.

package org.study.refactoringpractice.play;

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

public class Theater {

    private Map<String, Play> plays;

    public Theater(Map<String, Play> plays) {
        this.plays = plays;
    }

    public String statement(Invoice invoice) {
        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()) {

            int thisAmount = amountFor(performance);  <--- 필요 없어진 play 매개 변수 제거 

            volumeCredits += Math.max(performance.getAudience() - 30, 0);
            
            if ("comedy".equals(playFor(performance).getType())) {
                volumeCredits += Math.floor(performance.getAudience() / 5);
            }

            result += String.format("%s: %s (%d seats)\n", playFor(performance).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 Play playFor(Performance aPerformance) {
        return plays.get(aPerformance.getPlayID());
    }

    private int amountFor(Performance aPerformance) {  <--- 필요 없어진 play 매개변수 제거
        int result = 0;
        switch (playFor(aPerformance).getType()) {
            case "tragedy": // 비극
                result = 40_000;
                if (aPerformance.getAudience() > 30) {
                    result += 1000 * (aPerformance.getAudience() - 30);
                }
                break;
            case "comedy": // 희극
                result = 30_000;
                if (aPerformance.getAudience() > 20) {
                    result += 10_000 + 500 * (aPerformance.getAudience() - 20);
                }
                result += 300 * aPerformance.getAudience();
                break;
            default:
                throw new IllegalArgumentException(String.format("Unknown genre: %s", playFor(aPerformance).getType()));
        }
        return result;
    }
}

4. 마무리

오늘은 불필요한 매개변수인 play를 제거해봤습니다.

오늘 수행한 리팩터링에서 짚고 넘어가야 할 점은 성능입니다.

이전 코드는 반복을 돌 때마다 한 번만 공연을 조회했는데, 

리팩터링하고 나서는 반복을 돌 때마다 네 번이나 조회합니다.

뭔가 성능적으로 나빠진건 아닌가 의구심이 들 수 있습니다.

제대로 리팩터링된 코드는 그렇지 않은 코드보다 성능 개선이 훨씬 수월하다고 하니 

일단 진도를 끝까지 달려봅시다.

 

https://github.com/Griotold/refactoring-practice

GitHub - Griotold/refactoring-practice: 마틴 파울러 - 리팩토링 2판 - java로 연습