숫자 야구 게임과 눈물 없이 볼 수 없는 OOP 성장기

때는 6월 중순, 사내 스터디가 다시 시작되었다. 3명이라는 조촐한 인원이지만 전력은 이 정도

당연히 세번째가 나다. 😜

하나의 책을 선정해서 공부했던 작년과 달리 (나와 두분의) 수준 차이를 고려해 자율 모각코 + 그 주 한 것 발표하기로 진행되었다.

내 첫 과업은 약 1년 간 미뤄왔던 OOP로, 객체 지향 공부에는 이게 짱이라는 와잼님의 추천을 받아 넥스트 스텝에서 프리코스로 진행되었던 숫자 야구게임을 해보기로 했다.

넥스트 스텝에서는 유료로 코드 리뷰를 해주지만, 나는 다른 두 분의 스파르타 코드 리뷰를 받았다… ㅎㅎ

⇒ 실제 리뷰는 개발 관행에 맞게 매우 친절한 어조로 진행되었다. 😥😥

그리하여 이 포스팅은 눈물 없인 볼 수 없는 OOP 성장기라 할 수 있다.

들어가기 전

숫자 야구 게임 미션은 객체 지향과 클린 코드 지식 없이는 절대 풀 수 없다.

이 미션에는 두 가지 핵심 제약 사항이 있다.
1. 들여쓰기는 1까지만 허용한다. for문 안에 if문이 있으면 들여쓰기가 2가 된다. 즉 메소드 단위로 if문이나 for문 같은 statement는 딱 1개만 쓸 수 있다.
2. 한 메소드는 10줄이 넘어가서는 안된다.
3. 테스트 코드를 작성해야 한다.
이 제약 사항을 지키기 위해선 객체 지향적인 설계를 고려할 수 밖에 없다.

아무것도 몰랐던 1차 시도 (대실패)

첫번째 시도는 내가 그동안 해왔던 객체 지향이나 디자인 패턴 공부가 겉핥기 이론에 불과했다는 걸 깨닫는 계기가 된다.
야구 게임을 구현하는 로직 자체는 쉽다고 생각했다.
구조는 단일 패키지에 클래스 6개로 꽤 조촐하다.
테스트 코드는 작성하지 않고 만들었다. (어떻게 작성하는지 몰랐다)

웹 개발자 답게 Service와 Controller부터 만들고 시작했다. 내가 아는 클래스 역할 분리는 MVC 밖에 없었으므로…
나름 도메인의 역할을 한답시고 User객체와 Result객체가 있다.
도저히 1Depth 만으로 구성하지 못해 for문안에 if문을 썼다.

//매우 혹평을 받은 코드
private List<Integer> setBaseballNumber() {
        Set<Integer> baseballNumberSet = new HashSet<>(this.size);
        List<Integer> baseBallNumberList = new ArrayList<>(this.size);
        int size = 0;
        while(size <this.size) {
            int randomNumber = Randoms.pickNumberInRange(1, 9);
            if(**baseballNumberSet.add(randomNumber)**) {
                baseBallNumberList.add(randomNumber);
                size++;
            }
        }
        return baseBallNumberList;
    }

중복을 걸러내기 위한 Set과 실제 데이터가 들어가는 List를 사용했다.
if문 안에 add 코드는 가독성 면에서 최악의 평가를 받았다.
메소드 이름도 부적절하다는 평가를 받았다.
- 마치 setter같이 보이기 때문에 set 대신 generate 나 create가 적절

1차 코드 리뷰

두 가지 핵심 제약 사항이 모두 지켜지지 않았다. 이 미션의 의의를 잘 모르는 것 같다.
테스트 코드를 먼저 작성해야 한다.
테스트 코드를 작성하려면 기능을 작은 단위부터 만들어야 한다.
역할 별로 클래스를 나누고 책임을 분배하는 게 필요하다. (SRP : 단일 책임 원칙)
MVC 패턴에 얽매이지 마라.
view단은 어디? ⇒ 심지어 완성을 못함..
접근제어자가 무분별하게 사용된다. public과 private을 적절하게 사용해야 한다. (디미터의 법칙)
매직 넘버는 직접 작성하지 않고 상수로 빼서 관리하라
일급 컬렉션과 원시 타입 포장을 공부해라.
- 일급 컬렉션이란, 컬렉션을 한번 wrapping한 객체로 변수가 collection 타입 한 개만 존재하는 상태를 말한다.
- 원시 타입 포장은 마찬가지로 기본형을 한번 wrapping 하여 멤버 변수가 기본형 타입 한 개만 있는 상태를 말한다.
utils성 클래스들을 분리하라.

1차 후기

이게 왜 OOP를 요구하는지 전혀 이해하지 못한 채 만들었다.
‘야구 게임 구현’에 꽂혀서 실제 알고리즘 만들기에만 급급했다.
자세한 소스코드(개판)는 여기서 볼 수 있습니다.

감을 못 잡은 2차 시도 (여전히 실패)

1차 리뷰를 바탕으로 2차에 들어갔다.
패키지 구조

나름 클래스를 더 만들려고 용을 썼다.. (와중에 Service와 Controller는 없애버렸다.)
매직넘버를 관리하는 클래스를 만들었다. (BaseballGameSetting)
테스트 코드도 작성했지만, 방법을 몰라 하다 말았다.
여전히 2뎁스 코드가 존재한다.
리뷰의 흔적들

리뷰

실패 테스트를 해라
왜 HashSet이 아니라 LinkedHashSet을 이용했는지? ⇒ Integer 캐싱 때문에 set이 자꾸 오름차 순 정렬 되어버려서
의미 없는 파라미터가 아닌, 경계값 테스트를 해라. (문제의 소스 첨부)

실패 테스트를 해라
재귀 호출은 최악이다. (성능상으로도, 코드 가독성으로도)
- 왜 재귀 호출을 하면 안되는지 찾아봐라, stack overflow 와도 연관이 있다.
메소드 이름이 부적절하다. 예를 들면 숫자를 생성(generate)하는 메소드가 get으로 시작하면 생성이 아닌 존재하는 필드를 반환하는 getter같다. 메소드와 클래스 이름은 매우! 중요하다.
컬렉션과 배열의 변수명은 XXXList, XXXSet 이 아닌 영어 복수형을 사용해라.
- 리스코프 치환 원칙으로 List를 사용하다가도 언제든 필요에 의해 set으로 대체될 수 있다. 변화에 열려 있는 게 중요하다.
input과 output을 담당하는 클래스가 필요해보인다.
클래스 나누는 연습을 더 해야 한다.
Readme를 작성해서 어떤 기능이 필요한지 먼저 써보면 뭐가 필요한지 더 잘 알 수 있을 거다
일급 컬렉션이랑 원시 타입 포장 사용하라니까?

감이 안 오면 다른 사람 소스 코드를 읽어봐라.

2차 후기

나름 1차 피드백을 많이 반영했지만 여전히 혹평 투성이였다.
1차 리뷰를 받고도 전혀 감을 못 잡아서 속상했다.
나는 개발에 재능이 없는 건가…? 내가 이걸 만들 수는 있을까… 살짝 좌절감이 들었다. 조금 더 길게 얘기하자면, 국비 다닐 때도 나는 객체가 제일 어려웠다. 당시 강사님이 커피 주문 시스템 만들기를 미션을 해보라면서 몇 개의 클래스를 만들어주셨는데 당췌 ‘커피를 주문하다’ 라는 메소드는 어디에 만들어야 하는지 혼란스러웠던 기억이 있다. Order에 만들어야 하는가? Coffee에 만들어야 하는가?
도대체 어떻게 만들어야 한단 말인가? 어떤 클래스가 어떤 역할을 하는 기준은 무엇이란 말인가?
작년에 공부한 것들과 별개로 국비 시절에서 별로 성장하지 못했음을 느꼈다.
2차 리뷰가 끝난 후, 나 진짜 형편 없구나 + 그동안 했던 개발은 쓰레기였구나+ 약간의 자존감 하락을 얻었다.

클린 코드를 읽기 시작했다.

3차 시도 (약간의 진보)

3차 시도부터는 다른 사람의 소스 코드를 참고하기로 했다.
Readme를 작성했다.

클래스 구조

일급 컬렉션과 원시 타입 포장을 사용했다.
기존에 List<Intger>로 관리하던 야구 게임의 숫자를 일급 컬렉션으로 구성했다.
- Ball들을 비교하는 로직은 누구의 책임인가? ←이 부분이 가장 어려웠다.
  - ball들을 어떻게든 바깥에서 조작하기 위해 반복자 패턴을 사용했다. (틀린 접근 방식이었다.)

public class Balls {

    public final Set<Ball> balls;

    public Balls(final List<Integer> numbers) {
        validateSize(numbers);
        this.balls = generatedBalls(numbers);
        validateDuplicate(this.balls);
    }

    private void validateSize(final List<Integer> numbers) {
        if (numbers.size() != BaseballSetting.DIGITS) {
            throw new IllegalArgumentException(BaseballSetting.DIGITS + "개의 숫자를 입력해주세요");
        }
    }

    private void validateDuplicate(final Set<Ball> balls) {
        if (balls.size() != BaseballSetting.DIGITS) {
            throw new IllegalArgumentException("중복된 숫자는 입력할 수 없습니다.");
        }
    }

    private Set<Ball> generatedBalls(List<Integer> numbers) {
        Set<Ball> balls = new LinkedHashSet<>();
        for (Integer number : numbers) {
            balls.add(Ball.from(number));
        }
        return balls;
    }

    public int countBallTo(Balls balls) {
        return CollectionUtils.retailAllCount(this.balls, balls.balls);
    }

    public Iterator<Ball> createIterator() {
        return this.balls.iterator();
    }

}

원시 타입 포장으로 num (1~9까지의 수)를 검증 하는 책임을 num 클래스에 부여했다.

리뷰

코드가 많이 나아지긴 했다.
일급 컬렉션을 쓰긴 했지만 ‘왜’ 쓰는지 연구가 더 필요하다
Balls 와 PositionBalls의 아이디어는 좋았지만 꼭 필요했을까? 더 복잡해지기만 했다.

3차 후기

꽤 많은 고민을 했던 것 같다.
- 원시 객체 포장을 사용하면서 new로 생성되는데 1~9까지 미리 만들어두고 캐싱하면 좋지 않을까?
- random number는 꼭 1개씩 끊어서 만들어야 할까? 그냥 세자리 수를 뽑아내는 건 어떨까?
원시 객체 포장과 일급 컬렉션을 사용하니 validation 책임을 객체 스스로에게 맡겨 외부에서는 다른 로직 없이 그 객체를 믿고 사용할 수 있었다.
코드 리뷰에 좀 의연해졌다.
가장 힘든 점은 어떤 클래스에 어떤 책임과 행동을 부과해야 하는지였다.

이를 해소하기 위해 1년 전에 읽다 말았던 객체지향의 사실과 오해를 다시 읽기 시작했다.

4차 시도(완결)

객체지향의 사실과 오해에서 가장 와닿은 게 있다면, 클래스부터 정의하지 말고 행위를 먼저 정의하라는 것이었다.
이를 토대로 readme부터 다시 작성했다.

행위를 정의한 후에는 역할을 맡길 클래스들을 만들었다.

지금까지 없었던 Computer가 생겨났다. Computer가 생성한 숫자는 비교 기준이 된다.
- 컴퓨터는 숫자를 생성하며, balls을 가지고 있고, 숫자를 재생성하며, 숫자를 비교한다.

public class Computer {

    private final NumberGenerator numberGenerator;
    private Balls balls;

    public Computer() {
        this(new NonDuplicatedNumberGenerator());
    }

    public Computer(final NumberGenerator numberGenerator) {
        this.numberGenerator = numberGenerator;
        this.resetBalls();
    }

    public void resetBalls() {
        this.balls = new Balls(this.numberGenerator.generate(DIGITS));
    }

    public List<BaseballState> matchBalls(Balls anotherBalls) {
        List<BaseballState> baseballStates = new ArrayList<>();
        for(int i= 0 ; i < DIGITS ;i++) {
            baseballStates.add(this.balls.matchBall(anotherBalls.get(i)));
        }
        return baseballStates;
    }
}

User는 숫자를 입력하고, 볼을 만들며, 게임 재시작 여부를 결정한다.

public class User {

    private final UserNumberInput userNumberInput;

    public User(UserNumberInput userNumberInput) {
        this.userNumberInput = userNumberInput;
    }

    public Balls generatedBalls() {
        List<Integer> inputNumbers = userNumberInput.getInputNumbers();
        return new Balls(inputNumbers);
    }

    public int inputResetNumber() {
        int resetNumber = userNumberInput.getNumber();
        validateResetNumber(resetNumber);
        return resetNumber;
    }

    private void validateResetNumber(final int number) {
        if (!(number == NUMBER_TO_GAME_RESET || number == NUMBER_TO_GAME_END)) {
            throw new IllegalArgumentException(ExceptionMessage.NON_VALIDATED_RESET_NUMBER.getMessage());
        }
    }
}

두 객체가 어떤 식으로 숫자를 생성할지는 외부에서 주입 받아 결정된다.
외부 주입이 없으면 Default를 사용한다.
이 외의 클래스들은 두 클래스 들을 보조 하는 역할을 한다.
테스트는 가급적 모든 케이스 별로 하려고 했다.

class ResultTest {
    @Test
    @DisplayName("낫싱 테스트")
    void noting() {
        Result result = new Result(0, 0);
        String message = result.getMessage();
        Assertions.assertThat(message).isEqualTo("낫싱");
    }

    @Test
    @DisplayName("게임 종료 테스트")
    void 게임_종료() {
        Result result = new Result(3, 0);
        String message = result.getMessage();
        Assertions.assertThat(message).isEqualTo(BaseballGameMessage.ROUND_OVER.getMessage());
    }

    @Test
    @DisplayName("스트라이크 테스트")
    void 스트라이크_테스트() {
        strikeTest(1);
        strikeTest(2);

    }

    private void strikeTest(int strike) {
        Result result = new Result(strike, 0);
        String message = result.getMessage();
        Assertions.assertThat(message).isEqualTo(strike + "스트라이크");
    }

    @Test
    @DisplayName("볼 테스트")
    void 볼_테스트() {
        ballTest(1);
        ballTest(2);
        ballTest(3);
    }

    private void ballTest(int ball) {
        Result result = new Result(0, ball);
        String message = result.getMessage();
        Assertions.assertThat(message).isEqualTo(ball + "볼");
    }

    @Test
    @DisplayName("볼/스트라이크 테스트")
    void 볼_스트라이크_테스트() {
        ballAndStrikeTest(1, 1);
        ballAndStrikeTest(1, 2);
        ballAndStrikeTest(2, 1);
    }

    private void ballAndStrikeTest(int strike, int ball) {
        Result result = new Result(strike, ball);
        String message = result.getMessage();
        Assertions.assertThat(message).isEqualTo(ball + "볼 " + strike + "스트라이크");

    }

}

4차 리뷰

Output 부분을 static하게 구현했는데 view단에 해당하는 부분은 가장 바뀌기 쉬운 부분이므로 interface로 구현하는 걸 추천한다.
이 외의 리뷰들은 4차 코드만 대상으로 한 글을 따로 포스팅 하도록 하겠습니다

4차 후기

시간이 꽤 지나고 다시 코드를 보니 정말 많이 배웠음을 느낀다
코드는 결국 글과 같아서, 잘 읽혀야 한다는 게 제일 중요하다는 걸 깨달았다.
실제 미션을 진행하면서 책을 읽으니까 와닿는 부분이 많았다!
공부 방향성을 정하는 계기가 됐다!
코드 리뷰를 해주신 두 분께

지영이네 정상영업합니다