[리팩토링] 악취 4. 긴 매개변수 목록

 

✍️ 악취 4 : 긴 매개변수 목록

메서드의 매개변수가 많을수록 함수의 역할을 이해하기 어렵다. 고려해 볼 사항으로 과연 그 메서드가 한 가지 일을 하고 있는 게 맞는지, 불필요한 매개변수는 없는지, 논리적으로 그룹핑할 매개변수 목록은 없는지 검토해야 한다.

 

긴 매개 변수 목록을 위해 네 가지 리팩토링을 활용할 수 있다.

1. "매개 변수를 질의 함수로 바꾸기" 만약 어떤 매개변수를 다른 매개변수를 통해 알아낼 수 있다면

2. "플래그 제거하기" 매개 변수가 플래그로 사용된다면

3. "객체를 통째로 넘기기" 기존 자료구조에서 세부적인 데이터를 가져와서 플랫하게 넘기는 대신

4. "그룹핑해서 매개변수 객체 만들기" 일부 매개변수들이 반복적으로 넘겨진다면

 

🍊 매개 변수를 질의 함수로 바꾸기

메서드의 매개변수는 메서드를 이해하는데 굉장히 중요한 역할을 한다. 그렇기에 메서드의 매개변수가 많으면 많을수록 어떤 일을 하는지 추측하기 어려워진다. 만약 한 매개변수를 통해 다른 매개변수를 알아낼 수 있다면 "중복 매개변수"라고 한다.

 

메서드를 호출하는 측면에서도 고려해 보면, 결국 인자를 넘겨주는 것은 함수를 호출하는 쪽의 책임이다. 메서드 호출에 있어서 너무 많은 책임을 외부에 넘기는 것보단 정말 필요한 매개 변수만 받아서 메서드 내부에서 책임지도록 설계해야 한다.

// Before
public class Order {

    private int quantity;
    private double itemPrice;

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

    /**
     * 비용 계산 후 수량에 따른 할인 레벨을 결정하고, 할인된 가격 반환 
     * @return
     */
    public double finalPrice() {
        double basePrice = this.quantity * this.itemPrice;
        int discountLevel = this.quantity > 100 ? 2 : 1;

        return discountedPrice(basePrice, discountLevel);
    }

    /**
     * 할인 레벨에 따른 할인가격 계산
     */
    private double discountedPrice(double basePrice, int discountLevel) {
        return discountLevel == 2 ? basePrice * 0.90 : basePrice * 0.95;
    }
}

 

// After
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 discountedPrice(basePrice);
    }

    // discountLevel 계산을 discountedPrice의 책임으로 변경함
    private double discountedPrice(double basePrice) {
        return discountLevel() == 2 ? basePrice * 0.90 : basePrice * 0.95;
    }

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

 

🌱  플래그 인수 제거하기

일반적으로 플래그로 넘겨지는 매개변수는 메서드 내부 로직을 분기하기 위해 사용한다. 이처럼 코드에 종종 등장하지만 가장 큰 단점은 메서드의 호출부에서 그 용도를 파악하기 어렵다는 것이다.

 

가령 아래 메서드는 어떤 일을 하는지 짐작이 가지만

playGame(players);

플래그 인자가 추가된 순간 내부 구현을 찾아보기 전까진 차이점이 무엇인지 알 수 없다.

playGame(players, true);
playGame(players, false);

 

'조건문 분해하기'를 활용하면 플래그 인수를 제거할 수 있다.

@AllArgsConstructor
@Getter
public class Order {

    private DateTime placedOn;
    private String deliveryAddress;
}

// Before
public class Shipment {

    public DateTime getDeliveryDate(Order order, boolean isRush) {
        if (isRush) {
            int deliveryTime = switch (order.getDeliveryAddress()) {
                case "SEOUL" -> 1;
                case "INCHEON" -> 2;
                default -> 3;
            };

            return order.getPlacedOn().plusDays(deliveryTime);

        } else {
            int deliveryTime = switch (order.getDeliveryAddress()) {
                case "SEOUL" -> 2;
                case "INCHEON" -> 3;
                default -> 3;
            };

            return order.getPlacedOn().plusDays(deliveryTime);
        }
    }
}

@Test
void deliveryDate() {
    DateTime placedOn = new DateTime(2021, 12, 15, 0, 0);
    Order order = new Order(placedOn, "SEOUL");

    Shipment shipment = new Shipment();
    assertEquals(placedOn.plusDays(2), shipment.getDeliveryDate(order, false));
    assertEquals(placedOn.plusDays(1), shipment.getDeliveryDate(order, true));
}

 

switch문 로직을 메서드로 빼면 다음과 같은 형태가 되고

public class Shipment {

    public DateTime getDeliveryDate(Order order, boolean isRush) {
        if (isRush) {
            return getRushDeliveryDate(order);

        } else {
            return getRegularDeliveryDate(order);
        }
    }

    private DateTime getRushDeliveryDate(Order order) {
        int deliveryTime = switch (order.getDeliveryAddress()) {
            case "SEOUL" -> 1;
            case "INCHEON" -> 2;
            default -> 3;
        };

        return order.getPlacedOn().plusDays(deliveryTime);
    }

    private DateTime getRegularDeliveryDate(Order order) {
        int deliveryTime = switch (order.getDeliveryAddress()) {
            case "SEOUL" -> 2;
            case "INCHEON" -> 3;
            default -> 3;
        };

        return order.getPlacedOn().plusDays(deliveryTime);
    }
}

 

외부에선 조건에따라 추출한 메서드를 호출하면 더 이상 플래그를 사용하지 않아도 된다.

public class Shipment {

    public DateTime getRushDeliveryDate(Order order) {
        int deliveryTime = switch (order.getDeliveryAddress()) {
            case "SEOUL" -> 1;
            case "INCHEON" -> 2;
            default -> 3;
        };

        return order.getPlacedOn().plusDays(deliveryTime);
    }

    public DateTime getRegularDeliveryDate(Order order) {
        int deliveryTime = switch (order.getDeliveryAddress()) {
            case "SEOUL" -> 2;
            case "INCHEON" -> 3;
            default -> 3;
        };

        return order.getPlacedOn().plusDays(deliveryTime);
    }
}

@Test
void deliveryDate() {
    DateTime placedOn = new DateTime(2021, 12, 15, 0, 0);
    Order order = new Order(placedOn, "SEOUL");

    Shipment shipment = new Shipment();
    assertEquals(placedOn.plusDays(2), shipment.getRegularDeliveryDate(order));
    assertEquals(placedOn.plusDays(1), shipment.getRushDeliveryDate(order));
}

 

🍃  여러 함수를 클래스로 묶기

비슷한 매개변수를 여러 메서드에서 사용 중이고 해당 메서드들을 논리적으로 그룹핑 할 수 있다면 메서드를 모아 하나의 클래스로 만들 수 있다. 하나의 클래스로 만들고 공통 매개변수를 필드로 옮긴다면 매개변수의 수를 줄일 수 있다.

 

가령 Student 객체를 검증하고 저장하는 로직이 있다고 가정했을 때 validate 메서드들을 하나의 클래스로 빼낼 수 있다.

// Before
public class StudentService {

    private StudentDao studentDao;

    public void saveStudent(Student student) {
        validateName(student.getName(), student.getGender());
        validateScores(student.getKor(), student.getEng(), student.getMath());

        studentDao.insert(student);
    }

    private void validateName(String name, char gender) {
        if (name.isEmpty() || name.length() > 10)
            throw new IllegalArgumentException("Invalid name length : " + name.length());

        String prefix = "STD_" + gender;
        if (!name.startsWith(prefix))
            throw new IllegalArgumentException("Student name must start with " + prefix);
    }

    private void validateScores(int kor, int eng, int math) {
        boolean isValid = Arrays.asList(kor, eng, math).stream().noneMatch(v -> v < 0 || v > 100);
        if (!isValid)
            throw new IllegalArgumentException("Invalid Score");
    }
}

 

// After
public class StudentService {

    private StudentDao studentDao;

    public void saveStudents(Student student) {
        StudentValidator validator = new StudentValidator(student);
        validator.validateName();
        validator.validateScores();

        studentDao.insert(student);
    }
}

@AllArgsConstructor
public class StudentValidator {

    private Student student;

    public void validateName() {
        String name = student.getName();
        if (name.isEmpty() || name.length() > 10)
            throw new IllegalArgumentException("Invalid name length : " + name.length());

        String prefix = "STD_" + student.getGender();
        if (!name.startsWith(prefix))
            throw new IllegalArgumentException("Student name must start with " + prefix);
    }

    public void validateScores() {
        boolean isValid = Arrays.asList(student.getKor(), student.getEng(), student.getMath()).
                stream().noneMatch(v -> v < 0 || v > 100);
        if (!isValid)
            throw new IllegalArgumentException("Invalid Score");
    }
}