[Refactoring] 4. 긴 매개변수 목록

긴 매개변수 목록

• 어떤 함수에 매개변수가 많을수록 함수의 역할을 이해하기 어려움
  • 과연 그 함수는 한 가지 일을 하고 있는게 맞는가?
  • 불필요한 매개변수는 없는가?
  • 하나의 레코드로 뭉칠 수 있는 매개변수 목록은 없는가?
• 사용 가능한 리팩토링 기술
  • 어떤 매개변수를 다른 매개변수를 통해 알아낼 수 있다면?
    • => "매개변수를 질의함수로 바꾸기(Replace Paramter with Query)"
    • "임시 변수를 질의 함수로 바꾸기(Replace Temp with Query)"는 이전 시간에 이미 다루었다
  • 기존 자료구조에서 세부적인 데이터를 가져와서 여러 매개변수로 넘기는 대신,
    • => "객체 통째로 넘기기(Preserve Whole Object)"
  • 매개변수가 플래그로 사용된다면?
    • => "플래그 인수 제거하기(Remove Flag Argument)"
  • 여러 함수가 일부 매개변수를 공통적으로 사용한다면?
    • => "여러 함수를 클래스로 묶기(Combine Functions into Class)"
    • 매개변수를 해당 클래스의 필드로 만들고 메서드에 전달해야 하는 매개변수 목록을 줄일 수 있음
    • "매개변수 객체 만들기(Introduce Paramter Object)"와 동일!

매개변수를 질의 함수로 바꾸기(Replace Paramter 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;
    }
}

 

위 코드에서 discountLevel을 구한 후, discountedPrice() 메서드에 전달하고 있다. 여기에 "임시 변수를 질의 함수로 바꾸기" + "함수 선언 변경하기", 즉 "매개변수를 질의 함수로 바꾸기(Replace Paramter 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;
        return this.discountedPrice(basePrice);
    }

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

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

 

discountLevel을 계산하는 책임이 discountedPrice 메서드 내부로 옮겨지게 되었다

플래그 인수 제거하기(Remove Flag Argument)

• 플래그 <- 보통 함수에 매개변수로 전달해서, 함수 내부의 로직을 분기하는 데에 사용
• 플래그를 사용한 함수는 의미를 파악하기 어렵다..
  • 변경 전: bookConcert(customer, false), bookConcert(customer, true) -> 구현 보기를 전까지는 둘의 차이를 알기 힘듦
  • 변경 후: bookConcert(customer), premiumBookConcert(customer) -> 더 직관적
• 플래그의 단점
  • 플래그가 너무 많다면, 하나의 메서드 혹은 클래스가 너무 많은 책임을 가지고 있는 것
  • 플래그가 하나만 있더라도, 해당 메서드가 하는 일의 의미를 구현부를 보지 않으면 파악하기 힘들어짐
  • => 즉, 플래그는 있으면 무조건 리팩토링 대상이라고 봐도 무방
• 조건문 분해하기(Decompose Condition)를 활용할 수 있음
  • 플래그에 따라 달라지는 로직을 각각의 함수로 추출하여 이를 외부에서 호출하도록 변경

예제 코드

public class Order {

    private LocalDate placedOn;
    private String deliveryState;

    public Order(LocalDate placedOn, String deliveryState) {
        this.placedOn = placedOn;
        this.deliveryState = deliveryState;
    }

    public LocalDate getPlacedOn() {
        return placedOn;
    }

    public String getDeliveryState() {
        return deliveryState;
    }
}

public class Shipment {

    public LocalDate deliveryDate(Order order, boolean isRush) {
        if (isRush) {
            int deliveryTime = switch (order.getDeliveryState()) {
                case "WA", "CA", "OR" -> 1;
                case "TX", "NY", "FL" -> 2;
                default -> 3;
            };
            return order.getPlacedOn().plusDays(deliveryTime);
        } else {
            int deliveryTime = switch (order.getDeliveryState()) {
                case "WA", "CA" -> 2;
                case "OR", "TX", "NY" -> 3;
                default -> 4;
            };
            return order.getPlacedOn().plusDays(deliveryTime);
        }
    }
}

 

Shipment에서 deliveryDate 메서드의 파라미터로 isRush라는 플래그를 받고있는 것을 볼 수 있다.

public class Shipment {

    public LocalDate regularDeliveryDate(Order order) {
        int deliveryTime = switch (order.getDeliveryState()) {
            case "WA", "CA" -> 2;
            case "OR", "TX", "NY" -> 3;
            default -> 4;
        };
        return order.getPlacedOn().plusDays(deliveryTime);
    }

    public LocalDate rushDeliveryDate(Order order) {
        int deliveryTime = switch (order.getDeliveryState()) {
            case "WA", "CA", "OR" -> 1;
            case "TX", "NY", "FL" -> 2;
            default -> 3;
        };
        return order.getPlacedOn().plusDays(deliveryTime);
    }
}

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

• 비슷한 매개변수 목록을 여러 함수에서 사용하고 있다면? => 해당 메서드를 모아서 클래스를 만들 수 있음
• 클래스 내부로 메서드를 옮기고, 데이터를 필드로 만들면 메서드에 전달해야 하는 매개변수 목록도 줄일 수 있음
• '커맨드 패턴'을 적용했던 것과 굉장히 유사하다

관련 예제 코드는 이전 포스팅의 '커맨드 패턴' 부분에서 다루었기에 생략하겠다!