반복되는 유효성 체크와 같은 중복 로직은 어떻게 제거할 수 있을까?

2017-10-27 10:09

오늘 코드 리뷰 주제는 다음 그림과 같은 볼링 게임 점수판을 구현하는 프로그래밍의 일부이다.

bowling board

볼링 게임의 각 프레임을 Frame이라는 클래스로 추상화했다. Frame은 쓰러진 핀을 처리할 수 있는 bowl()이라는 메소드를 다음과 같이 가진다.

public class Frame {
    [...]

    public Frame bowl(int countOfPin) {
      if (countOfPin < 0 || countOfPin > 10 ) {
         throw new IllegalArgumentException(String.format("쓰러진 볼링 핀은 0 이상 10 이하여야 합니다. 현재 값은 : %d", countOfPin));
      }

      state = state.bowl(countOfPin);
      if (state.isFinish()) {
        next = new Frame(no + 1);
        return next;
      }
      return this;
    }
}

쓰러진 볼링 핀의 수는 0 이상, 10 이하의 값을 가져야 하기 때문에 이에 대한 방어 코드를 위와 같이 구현했다.

스페어를 추상화한 Spare 코드를 보면 다음과 같다.

class Spare extends Finished {
    private int first;
    private int second;

    Spare(int first, int second) {
      if (first + second != 10) {
        throw new IllegalArgumentException();
      }
      
      this.first = first;
      this.second = second;
    }

    [...]
}

그런데 위와 같이 방어 코드를 구현할 경우 first값이 20, second 값이 -10일 경우에도 요구사항을 만족한다. 따라서 이에 대한 방어 코드를 구현하려면 다음과 같이 구현해야 한다.

class Spare extends Finished {
    private int first;
    private int second;

    Spare(int first, int second) {
        validPin(first);
        validPin(second);
        
        if (first + second != 10) {
          throw new IllegalArgumentException();
        }
        
        this.first = first;
        this.second = second;
    }
    
    private void validPin(int countOfPin) {
        if (countOfPin < 0 || countOfPin > 10 ) {
            throw new IllegalArgumentException(String.format("쓰러진 볼링 핀은 0 이상 10 이하여야 합니다. 현재 값은 : %d", countOfPin));
        }
    }
}

이처럼 프로그래밍을 하다보면 방어 코드를 구현하는데 상당히 많은 중복 코드가 발생하는 것을 경험하게 된다. 이 중복 코드를 어떻게 제거하는 것이 좋을까?

힌트는 객체지향 생활체조 원칙의 "규칙 3: 원시값과 문자열의 포장"을 적용하는 방법을 검토해 보자.

위 샘플 예제는 코드스쿼드 에서 새롭게 진행 중인 마스터즈 코스에서 발췌한 코드입니다. 코드스쿼드의 마스터즈 코스는 코드 리뷰 방식의 개인별 맞춤 학습 방법입니다.

0개의 의견 from FB

1개의 의견 from SLiPP

2017-10-27 17:17

이 같은 상황의 문제점은 int 값이 코드 전반적으로 사용된다는 것이다. 볼링 게임에서 쓰러진 핀의 수는 0에서 10까지만 허용가능하다. 그런데 메소드의 인자로 int를 받는 경우 매번 0에서 10까지의 값인지를 체크해야 한다는 것이다.

이 문제를 해결하려면 0에서 10까지의 값을 추상화한 클래스를 추가해 해결할 수 있다. 이 클래스를 Pins라는 클래스로 다음과 같이 구현할 수 있다.

public class Pins {
    private static final int MIN_PINS = 0;
    public static final int MAX_PINS = 10;
    
    private int falledPins;

    private Pins(int falledPins) {
        validPins(falledPins);
        this.falledPins = falledPins;
    }
    
    private static void validPins(int falledPins) {
        if (falledPins > MAX_PINS) {
            throw new IllegalArgumentException(
                    String.format("볼링 핀은 최대 10을 넘을 수 없습니다. 현재 쓰러진 핀 수는 %d", falledPins));
        }
        
        if (falledPins < MIN_PINS) {
            throw new IllegalArgumentException(
                    String.format("볼링 핀은 최초 0 미만이 될 수 없습니다. 현재 쓰러진 핀 수는 %d", falledPins));
        }
    }

    public static Pins bowl(int falledPins) {
        return new Pins(falledPins);
    }

    [...]
}

이와 같이 Pins 클래스로 추상화한 후 기존에 int로 사용하던 값을 Pins를 사용하도록 하면 Pins가 0에서 10의 값을 가지는 것이 보장되는 것이다.

    public Frame bowl(int countOfPin) {
        state = state.bowl(Pins.bowl(countOfPin));
        if (state.isFinish()) {
            next = new Frame(no + 1);
            return next;
        }
        return this;
    }
class Spare extends Finished {
    private Pins firstPins;
    private Pins secondPins;

    public Spare(Pins firstPins, Pins secondPins) {
        if (!firstPins.isSpare(secondPins)) {
            throw new IllegalArgumentException();
        }
        
        this.firstPins = firstPins;
        this.secondPins = secondPins;
    }

    [...]
}

위와 같이 최초로 int를 받는 곳에서 Pins로 변환한 후 이후 모든 구현을 int가 아닌 Pins를 사용하도록 한다면 훨씬 더 안정적으로 서비스를 구현할 수 있다. 또한 예외적인 값이 전달되는 상황에 대한 에러 처리에 대한 중복 코드 또한 제거할 수 있다.

의견 추가하기

연관태그

← 목록으로