[아무튼 필사] 내 코드가 그렇게 이상한가요? 5일차

5장 응집도 : 흩어져 있는 것들

5.5 매개변수가 너무 많은 경우

매개변수가 너무 많은 메서드는 응집도가 낮아지기 쉽습니다.

게임의 매직포인트(MP)를 예로 설명하겠습니다. PRG 같은 게임에는 매직포인트라는 개념이 있습니다. 이와 관련해 다음과 같은 사양이 있다고 합시다.

• 마법을 사용하면 매직포인트가 일정량 감소합니다.
• 회복 아이템 등을 사용하면 매직포인트가 일정량 회복됩니다.
• 매직포인트에는 최댓값이 있습니다.
• 매직포인트는 최댓값까지만 회복될 수 있습니다.
• 일부 장비는 매직포인트 최댓값을 높이는 효과가 있습니다.

설계를 따로 생각하지 않으면, 코드 5.23과 같은 로직이 만들어지기 쉽습니다.

/**
 * 매직포인트 회복하기
 * @param currentMagicPoint 현재 매직포인트 잔량
 * @param originalMaxMagicPoint 원래 매직포인트 최댓값
 * @param maxMagicPointIncrements 장비로 증가하는 매직포인트 최댓값 증가량
 * @param recoveryAmount 회복량
 * @return 회복 후의 매직포인트 잔량
 */
int recoverMagicPoint(int currentMagicPoint, int originalMaxMagicPoint, List<Integer> maxMagicPointIncrements, int recoveryAmount) {
    int currentMaxMagicPoint = originalMaxMagicPoint;
    for (int each : maxMagicPointIncrements) {
        currentMaxMagicPoint += each;
    }
    return Math.min(currentMagicPoint + recoveryAmount, currentMaxMagicPoint);
}

 

...

이 메서드는 정상적으로 기능하지만, 구조가 좋지 않습니다.

매직포인트 잔량, 최댓값, 장비 착용으로 인한 최댓값 증가량, 회복량을 각각 하나의 매개변수로 전달받습니다. 이렇게 너무 많은 매개변수를 받는 메서드는 실수로 잘못된 값을 대입할 가능성이 높습니다. 현재 예시 상황에서는 4개 정도의 데이터이지만, 실제 게임과 일반 애플리케이션에서는 굉장히 많은 값을 다룰 것입니다. 그래서 이렇게 너무 많은 매개변수를 받는 메서드는 다양한 문제를 일으킵니다.

또한 회복 이외의 기능도 수행하고 있습니다. 매직포인트 최댓값 증가량 계산은 회복 이외의 상황에도 사용하는 경우가 많을 것입니다. 이렇게 로직을 그대로 적으면, 중복 코드가 발생할 가능성이 높아집니다.

이러한 문제는 왜 생기는 것일까요? 메서드에 매개변수를 전달한다는 것은 해당 매개변수를 사용해서 어떤 기능을 수행하고 싶다는 의미입니다. 그래서 매개변수가 많다는 것은 많은 기능을 처리하고 싶다는 의미가 됩니다. 하지만 처리할 게 많아지면 로직이 복잡해지거나, 중복 코드가 생길 가능성이 높아집니다. 여러 악마들이 모여 많은 문제를 일으킬 것입니다.

 

5.5.1 기본 자료형에 대한 집착

boolean, int, float, double, String처럼 프로그래밍 언어가 표준적으로 제공하는 자료형을 기본 자료형(primitive type)이라고 합니다.

코드 5.23의 recoverMagicPoint 메서드와 마찬가지로 코드 5.24에 있는 discountPrice 메서드는 매개변수와 리턴 값에 모두 기본 자료형만 쓰고 있습니다. 이처럼 기본 자료형을 남용하는 현상을 기본 자료형 집착(primitive obsession)이라고 합니다.

/**
 * 
 * @param regularPrice 정가 
 * @param discountRate 할인율
 * @return 할인 가격
 */
int discountedPrice(int regularPrice, float discountRate) {
    if (regularPrice < 0) {
        throw new IllegalArgumentException();
    }

    if (discountRate < 0.0f) {
        throw new IllegalArgumentException();
    }

 

프로그래밍 초보자, 혹은 경력이 많더라도 줄곧 기본 자료형만을 써 온 개발자는 클래스 설계를 고려하지 않는 경우가 많습니다. 그래서 기본 자료형 집착에 빠지기 쉽습니다.

'아니, 이 정도를 집착이라기 할 수 있나? 일반적인 구현 스타일인 것 같은데?' 또는 '클래스를 많이 만드는 것이 오히려 이상해 보이는데?'라고 생각하는 독자가 있을 수도 있습니다. 하지만 잘못된 생각입니다. 코드 5.25를 살펴봅시다.

/**
 *
 * @param regularPrice 정가
 * @return 적절한 가격이라면 true
 */
boolean isFairPrice(int regularPrice) {
    if (regularPrice < 0) {
        throw new IllegalArgumentException();
    }

 

isFairPrice는 적절한 가격인지 확인하는 메서드입니다. 그런데 discountedPrice처럼 정가 regularPrice가 유효한 값인지 검사하는 코드가 여기도 있습니다. 기본 자료형으로만 구현하면, 이처럼 중복 코드가 많이 생깁니다. 또한 계산 로직이 이곳저곳에 분산되기 쉽습니다.

물론 기본 자료형만으로도 '동작하는 코드'를 작성할 수 있습니다. 하지만 그렇게 구현하면, 관련 있는 데이터와 로직을 집약하기 힘듭니다. 따라서 버그가 생기기 쉽고, 가독성이 떨어집니다.

데이터는 단순히 존재하기만 할 수는 없습니다. 데이터를 사용해 계산하거나 데이터를 판단해서 제어 흐름을 전환할 때 사용됩니다. 기본 자료형으로만 하면, 데이터를 사용한 계산과 제어 로직이 모두 분산됩니다. 응집도가 낮은 구조가 되는 것입니다.

...

코드 5.26처럼 할인 요금, 정가, 할인율을 하나하나의 클래스로 발전시켜 봅시다. 정가 클래스 RegularPrice 내부에 유효성 검사를 캡슐화했습니다. 할인율도 마찬가지로 클래스로 만들었습니다.

 

public class RegularPrice {
    final int amount;

    /**
     * @param amount 금액
     */
    RegularPrice(final int amount) {
        if (amount < 0) {
            throw new IllegalArgumentException();
        }
        this.amount = amount;
    }
  
}

 

그리고 할인 요금 DiscountedPrice에는 정가 클래스 RegularPrice와 할인율 클래스 DiscountRate를 전달합니다. 코드 5.24의 Common.discountedPrice와 다르게 매개변수가 기본 자료형이 아니라, 클래스로 바뀌었습니다.

public class DiscountedPrice {
    final int amount;

    /**
     * @param regularPrice 정가
     * @param discountRate 할인율
     */
    DiscountedPrice(final RegularPrice regularPrice, final DiscountRate discountRate) {
        // regularPrice와 discountRate를 사용해서 계산
    }

 

이렇게 하면 관련 있는 로직을 각각의 클래스에 응집할 수 있습니다.

5.5.2 의미 있는 단위는 모두 클래스로 만들기

...

매개변수가 너무 많아지는 문제를 피하려면, 개념적으로 의미 있는 클래스를 만들어야 합니다. 일단 매직포인트가 중심 개념입니다. 매직포인트를 나타내는 클래스 MagicPoint를 준비합니다. 그리고 매직포인트와 관련된 값들을 인스턴스 변수로 갖게 구성합니다.

/** 매직포인트 */
public class MagicPoint {
    // 현재 잔량
    int currentAmount;
    // 원래 최댓값
    int originalMaxAmount;
    // 장비 착용에 따른 최댓값 증가량
    List<Integer> maxIncrements;
}

 

그런데 이렇게 코드를 만들면 매직포인트 최댓값 계산 로직과 매직포인트 회복 로직이 다른 클래스에도 작성될 수 있습니다. 

따라서 매직포인트 최댓값 계산과 회복 메서드를 MagicPoint 클래스에 정의합니다. 이때 다른 클래스에서 불필요한 조작을 하지 못하게, 인스턴스 변수는 private으로 만듭니다. 이외에도 매직포인트 소비 메서드 등도 정의합니다.

 

/** 매직포인트 */
public class MagicPoint {
    // 현재 잔량
    private int currentAmount;
    // 원래 최댓값
    private int originalMaxAmount;
    // 장비 착용에 따른 최댓값 증가량
    private List<Integer> maxIncrements;
    
    // 생략
    
    /** @return 현재 매직포인트 잔량 */
    int current() {
        return currentAmount;
    }

    /** @return 매직포인트 최댓값 */
    int max() {
        int amount = originalMaxAmount;
        for (int each : maxIncrements) {
            amount += each;
        }
        return amount;
    }

    /** 
     * 매직포인트 회복하기
     * @param recoveryAmount 회복량
     */
    void recover(final int recoveryAmount) {
        currentAmount = Math.min(currentAmount + recoveryAmount, max());
    }

    /**
     * 매직포인트 소비하기
     * @param consumeAmount 소비량
     */
    void consume(final int consumeAmount) { ... }
}

 

매직포인트와 관련된 로직이 클래스 안에 잘 응집되었습니다.

매개변수가 많으면 데이터 하나하나를 매개변수로 다루지 말고, 그 데이터를 인스턴스 변수로 갖는 클래스를 만들고 활용하는 설계로 변경해 보세요.

 

---

 

오늘은 내용이 좀 길어졌다.

아무래도 공감가는? 내용이다 보니 한글자도 빼먹고 싶지 않았던 것 같다 ㅎㅎ

 

5일 동안 개발서적을 필사하면서 나쁜 구조를 어떻게 개선할 수 있는지 알아보았다.

일반적인 필사와는 좀 다르게 맥락이 이해가 가야하는 것 때문에 많은 내용을 옮겨적어야 했는데...

책 내용을 그대로 옮기는 것이 문제가 될 수 있을 것 같기도 하고,

감질맛 나게 끊어서? 직접 책을 보시라는 의미로? '내 코드그 그렇게 이상한가요?' 필사는 오늘까지만 하려고 한다.

남은 주말동안은 내가 그동안 읽었던 책들 중 인상깊었던 구절들을 따로 빼놓은 것들이 몇개 있는데,

그것들을 따라 써보려고 한다.

남은 챌린지도 화이팅

우리 존재 화이팅

---

출처

내 코드가 그렇게 이상한가요?