[우테코 5기] 사다리 타기 게임 미션 회고

후추랑 진행한 두 번째 페어 프로그래밍인 사다리 미션이다.
사실 이번 미션은 굉장히 빨리 merge가 되어서 미션 자체에 많이 신경을 쓰지는 않았던 것 같은데, 다른 크루분들이 고민하시는 걸 보고 있으니 내가 너무 생각을 안 한 것인가... 라는 생각이 들었다 💦 다음 미션에서는 조금 더 신경쓰면서 진행해야 할 것 같다.
그래도 후추 덕분에 도메인 설계를 굉장히 수월하게 해서, 더 빠르게 마무리 할 수 있었던 시간인 것 같다.
 
(물론... 이 게시글을 작성한 시점에서는 한참 전에 끝난 미션이지만, 임시 저장을 해두고 올리지 않았어서...ㅎ 뒤늦게 올린다)

 

✔️ 작성한 코드

GitHub - Cl8D/java-ladder: 사다리타기 미션을 위한 저장소

✔️ 1차 PR

[1단계 - 사다리 생성] 져니(이지원) 미션 제출합니다. by Cl8D · Pull Request #140 · woowacourse/java-ladder

✔️ 2차 PR

[2단계 - 사다리 게임 실행] 져니(이지원) 미션 제출합니다. by Cl8D · Pull Request #156 · woowacourse/java-l

 

---

✔️ 기능 요구사항

- 사다리 게임에 참여하는 사람에 이름을 최대 5글자까지 부여할 수 있다.
- 사다리를 출력할 때 사람 이름도 같이 출력한다.
- 사람 이름은 쉼표(,)를 기준으로 구분한다.
- 사람 이름을 5자 기준으로 출력하기 때문에 사다리 폭도 넓어져야 한다.
- 사다리 타기가 정상적으로 동작하려면 라인이 겹치지 않도록 해야 한다.
     - |-----|-----| 모양과 같이 가로 라인이 겹치는 경우 어느 방향으로 이동할지 결정할 수 없다.
- 사다리 실행 결과를 출력해야 한다.
- 개인별 이름을 입력하면 개인별 결과를 출력하고, "all"을 입력하면 전체 참여자의 실행 결과를 출력한다.

---

 
PR 내용 중에서 몇 가지 인상 깊었던 내용을 적어보려고 한다.
 

💬 사다리는 완전히 랜덤하게 생성되어야 할까?

사다리 게임에서 사다리가 랜덤으로 생성되는 게 중요할까? 사실 페어 프로그래밍을 하기 전에는 전혀 고려하지 않았을 사항이지만, 후추가 의문을 제기해 주었다. 듣고 보니 사다리 게임에서 어느 라인을 고르든 동일한 확률로 다른 곳에 도달해야 된다고 생각했기 때문이다. 그러나, 이 조건을 만족하기 위해서는 너무 복잡한 로직이 들어가야 했다. 

리뷰어님께 바로 물어본 결과, 사다리 게임에서 과연 그게 중요한 것인가?를 따져보라고 하셨다. 사실 게임에서 가장 중요한 것은 시작지에서 다른 도착지로 가는 것이 중요한 거였으니까. 동시에 다음과 같은 영상을 주셨는데, 애초부터 사다리 게임은 불공평하다고 한다 ㅎㅎ

---

💬 Scanner vs BufferedReader

    private String readConsole() {
        try {
        return bufferedReader.readLine();
        } catch (IOException e) {
        OutputView.print(e.getMessage());
        }
        return null;
        }

사용자에게 입력받을 때 왜 BufferedReader를 사용하는지에 대한 질문을 들었었다. 사실, 나는 코딩 테스트를 하면서 성능에 유난히 신경쓰다 보니까 Scanner보다 BufferedReader를 일종의 습관처럼 사용했었는데, 왜 성능이 더 좋은지 제대로 찾아본 적이 없었다. 그래서 이번 기회에 궁금해서 찾아보았다.
 
Scanner의 경우, 데이터를 입력받은 즉시 사용자에게 전송하며, 내부에서 정규식을 사용하다 보니 시간이 더 걸리고, BufferedReader의 경우 사용자에게 바로 전송하지 않고, 버퍼에 저장해두었다가 개행이나 가득 찼을 경우 사용자에게 전달하여 처리 시간이 더 적게 걸린다. 그 외에도 예외 처리를 진행해줘야 한다는 점, 별도의 타입에 따른 파싱이 필요하다는 점이 있다.
 
리뷰어님이 직접 테스트도 진행해 주셨는데, 다음과 같이 결과가 나왔다!

확실히 성능적으로는 좋지만, 사실 이번 미션에서는 오히려 독이 되었을 수도 있다. 명시적으로 try-catch를 통해서 IoException을 잡아야 했기 때문에 오히려 코드가 더 안 예쁘게 보일 수도 있었다는 점이다. 하지만, 개인적으로는 성능을 조금 더 중요시하는 입장에서 리뷰어님께 다음과 같이 말씀드렸다.

대충 checked Exception은 unchecked Exception으로 감싸는 걸 선호한다는 이야기... 아무튼, 그래서 리팩터링은 진행하지 않았지만 이번 기회에 새롭게 알아갈 수 있었어서 도움이 되었다.

---

💬 정보전문가 패턴

이것도 리뷰어님이 주셨던 레퍼런스 중 하나이다. 어떠한 행위에 대해서, 가장 그 행위를 잘할 것 같은 객체에게 위임하는 것이다!
이 시점에서 나는 사용자에게 입력받는 부분에 대해, 뷰와 컨트롤러를 다음과 같이 구성하였다.

public class InputView {

    private final BufferedReader bufferedReader;

    public InputView() {
        this.bufferedReader = new BufferedReader(new InputStreamReader(System.in));
    }

    public String readConsole() {
        try {
            return bufferedReader.readLine();
        } catch (IOException e) {
            OutputView.print(e.getMessage());
        }
        return null;
    }

    public <T> T getUserInput(Supplier<T> inputReader) {
        try {
            return inputReader.get();
        } catch (IllegalArgumentException e) {
            OutputView.print(e.getMessage());
            return getUserInput(inputReader);
        }
    }
}

// Controller
private Participants createParticipants() {
        return inputView.getUserInput(() -> {
        OutputView.print(INPUT_PARTICIPANT_NAMES_GUIDE.getMessage());
        String participantNames = inputView.readConsole();
        return Participants.create(participantNames);
        });
        }

입력하는 안내 메시지를 컨트롤러에서 보여주고, 단순히 뷰에서는 입력만 받았던 것이다. 사실 뷰의 메서드를 재사용할 수 있었기 때문에 위와 같은 방식을 진행했었지만, 정보 전문가 패턴의 관점으로 바라보았을 때는 다른 개발자가 '사용자에게 안내 메시지를 출력하고, 메시지를 입력받는 기능에 대해 어느 패키지를 먼저 열어볼 것인가?'라고 따져보면 좋은 구조는 아니다. InputView를 먼저 보는 개발자들도 분명 존재할 것이기 때문이다.
 
그래서, 다음과 같이 리팩터링을 진행했다.

public String getParticipantNames() {
    OutputView.print(INPUT_PARTICIPANT_NAMES_GUIDE.getMessage());
    return readConsole();
}

 // Controller
private Participants createParticipants() {
    return inputView.getInputWithRetry(() -> {
        String participantNames = inputView.getParticipantNames();
        return Participants.create(participantNames);
    });
}

이렇게 되면 입력받는 기능에 따라 메서드가 늘어난다는 점이 있지만, '해당 행위가 어디의 책임인가?'의 관점에서 보았을 때, 사용자에게 메시지를 출력하고 입력받는 기능까지 뷰가 담당하게 되어 책임 분리를 잘 한 것 같다는 생각이 든다.

---

💬 사다리 생성 로직

당시에는 오버 엔지니어링에 무척 신경쓰고 있었다. 수업을 들으면서 오버 엔지니어링은 오히려 악영향이라는 생각을 많이 했기 때문이다. 하지만, OCP를 지키기 위해 CBP에 대해 리뷰어님이 말씀해 주셨다 :D 모든 확장성에 대해서 고려하기는 힘들다는 것이다. 동시에 제너레이터에 대해서도 말씀해 주셨는데, 이는 아래에 후술하겠다.
 
우선, 기존의 사다리 생성 로직이다.

public class Ladder {

    private final List<Rungs> ladder;

    public Ladder(final List<Rungs> ladder) {
        this.ladder = ladder;
    }
}

public class LadderFactory {
    private static final int MIN_HEIGHT = 1;
    private static final int MAX_HEIGHT = 10_000;
    private static final int DEFAULT_COUNT = 1;
    private final NumberGenerator rungNumberGenerator;

    public LadderFactory(final NumberGenerator rungNumberGenerator) {
        this.rungNumberGenerator = rungNumberGenerator;
    }

    public Ladder createLadder(final String height, final int participantCount) {
        int ladderHeight = convertLadderHeight(height);
        List<Rungs> ladder = new ArrayList<>();
        for (int i = 0; i < ladderHeight; i++) {
            ladder.add(new Rungs(makeRungCount(participantCount), rungNumberGenerator));
        }
        return new Ladder(ladder);
    }
}

사다리의 생성 로직을 사다리가 가지는 것이 아닌, 별도의 객체에게 책임을 할당하였다.
이는 사다리를 생성하기 위해 필요한 요구사항이 변경될 수도 있다는 개념과, 사다리를 생성하는 규칙 자체가 상당히 복잡했기 때문에 이를 분리하고 싶었다. 또한, 조금 더 동적으로 생성하도록 만들고 싶어서 height와 participantCount를 변수로 관리하지 않고 인자로 받아서 조건에 맞게 사다리를 생성할 수 있도록 하고 싶었다.
 
그러나, 위 코드는 치명적인 실수가 있다. LadderFactory가 Ladder를 생성하기 위해서 Ladder의 생성자가 열리게 되는 것이다. 당시, 처음으로 코드를 작성할 때는 접근제어자 마저도 public으로 열어서 외부에서 Ladder를 통해 '사다리'라는 객체를 만들 위험성도 존재하게 된 것이다.

당시 리뷰어님께 받았던 피드백이다. 나는 LadderFactory를 통해서 Ladder를 생성하기 원했지만, 다른 개발자는 그걸 알 턱이 없는 것이다. 하지만 주석을 통해 이러한 세부 계약을 관리하는 것은 좋지 않다. 그래서 처음에는 Ladder의 생성자를 default 접근 제어자로 막는 식으로 구현하였다. 
 
하지만... 사실 접근 제어자도 결국 같은 패키지 내에서는 생성을 막을 수 없게 된다. 물론, 패키지 분리를 잘해서 domain 영역 - Ladder에 관한 영역으로 나누고 엄격하게 관리하면 되겠지만, 이 역시 결국 개발자가 관리해야 하는 일이고, 다른 협업자가 패키지를 이동하기라도 한다면 결국 Ladder는 다른 방식으로 생성될 수도 있게 된다.

그림으로 생각한다면 현재 이런 느낌인 것이다.
 
그래서...! 리뷰어님이 정말 좋은 방안을 주셨다 👍 

public class Ladder {
    private final List<Rungs> ladderRungs;
    private Ladder(final List<Rungs> ladderRungs) {
        this.ladderRungs = ladderRungs;
    }

    public static Ladder create(final String height, final int participantCount, final BooleanGenerator rungGenerator) {
        List<Rungs> ladderRungs = LadderRungsFactory
                .getInstance(rungGenerator)
                .createLadderRungs(height, participantCount);
        return new Ladder(ladderRungs);
    }
}

public class LadderRungsFactory {

    private static final int MIN_HEIGHT = 1;
    private static final int MAX_HEIGHT = 10_000;
    private static final int DEFAULT_COUNT = 1;
    private static final Map<BooleanGenerator, LadderRungsFactory> CACHE = new HashMap<>();

    private final BooleanGenerator rungGenerator;

    private LadderRungsFactory(final BooleanGenerator rungGenerator) {
        this.rungGenerator = rungGenerator;
    }

    static LadderRungsFactory getInstance(final BooleanGenerator rungGenerator) {
        if (CACHE.containsKey(rungGenerator)) {
            return CACHE.get(rungGenerator);
        }

        LadderRungsFactory ladderRungsFactory = new LadderRungsFactory(rungGenerator);
        CACHE.put(rungGenerator, ladderRungsFactory);
        return ladderRungsFactory;
    }

    protected List<Rungs> createLadderRungs(final String height, final int participantCount) {
        int ladderHeight = convertToLadderHeight(height);
        List<Rungs> ladder = new ArrayList<>();
        for (int i = 0; i < ladderHeight; i++) {
            ladder.add(Rungs.create(makeRungCount(participantCount), rungGenerator));
        }
        return ladder;
    }

무슨 차이가 있는지 눈에 보이는가? (캐시는 팩토리 자체가 여러 번 생성될 필요가 없기 때문에 처리해두었다.)
사다리를 생성할 때 단순히 Ladder 객체를 Ladder가 혼자 생성하는 것이 아닌, 내부적으로 LadderFactory를 호출하여 생성하는 것이다. 제어 로직이 Ladder -> LadderFactory -> Ladder로 다시 위임하는 구조가 되는 것이다.
 
그림으로 살펴보자. 캐싱에 대한 부분은 제외하고 로직을 다시 세웠다.

우선, LadderFactory가 필요한 변수의 경우 Ladder가 가지게 된다. 
Ladder는 사다리를 생성할 때 내부적으로 LadderFactory를 호출하여 사다리 생성에 필요한 부품인 List<Rungs>를 받게 되고, 같은 패키지 내에서 LadderFactory의 createLadderRungs를 호출하더라도 Ladder 자체를 생성할 수 없기 때문에 결국 'Ladder' 자체를 생성하기 위해서는 Ladder 객체를 생성하게 된다. 
 
위 해법을 알고 나서 정말 큰 깨달음을 얻게 되었다... 이는 추후 나올 체스 미션에서도 알차게 써먹었다.

---

💬 예외 메시지 처리

사실 예외 메시지는 이전 미션에서도 계속 고민해왔었다. 이전까지는 메시지 자체를 enum으로 관리하면서 별도의 예외 메시지에 대해 한 번에 관리하도록 만들었었는데, 서비스가 커지고 도메인이 커진다면 이런 식으로 관리하는 게 그렇게 좋지 않을 것 같다는 생각이 들었다. 

public enum ErrorMessage {
        AAA("")
    ...
    }

하지만, 이런 util성 객체가 과연 객체지향적 관점에서 유의미할 것인가?를 생각해보아야 한다.
나는 이전까지 util 성 객체에 대해서 아무 생각도 없었다. 그래서 이번 기회에 한 번 생각해보았다...

어느 곳에서 참조가 가능한 클래스라면 객체의 책임 분리 관점에서 맞지 않다는 것으로 결론을 내렸다. (SRP 위반)
그래서 결과적으로는 Custom Exception을 생성해서 예외가 발생한 지점에서 메시지를 관리하는 방향으로 변경하였다.

도메인에서 커스텀 예외를 발생시키는 방향으로 진행하셨군요!
그럼 도메인을 참조한 객체 입장에서 커스텀 예외를 구분할 수 있는 기준이 생겼습니다 ^_^ 해당 오류가 발생할 수 있는 지점에서 특정 타입으로 Catch 할 수 있으니까요
그러면 특정한 도메인 예외를 Catch해서 다른 메시지로 바꿔 출력할 수도 있겠죠?
더 나아가 도메인 예외를 그룹핑 (RuntimeException을 상속하는 ServiceException을 만들고 각 도메인 예외는 ServiceException을 상속)하는 식으로 "특정 그룹의 예외는 A 메시지로 일괄처리" 같은 로직도 만들 수 있겠습니다.

리뷰어님 최고... 👍
하지만 이런 관점에서 생각해보았을 때 모든 도메인마다 예외 메시지를 분리해서 관리해야 할까? 라는 문제도 생긴다.
예외 메시지에 대한 클래스가 10개, 20개...로 늘어나게 된다면 결국 그거에 대한 관리 비용도 늘어나기 때문이다.
그래서 처음에는 범용적으로 사용할 수 있도록 만들었으나, 리뷰어님은 오히려 여러 개가 있어도 상관없다고 말씀해 주셨다.
 
✨ 그래서 나도 나만의 기준을 세웠다.
1. 큰 범위의 예외 만들기
2. 내부적으로 해당 예외를 상속받은 커스텀 예외를 만들기
- 이때, 예외 메시지는 예외 발생 지점에서 관리하도록 만들기
- 예외 메시지가 변경되면 테스트 코드도 바뀌겠지만, 프로덕션 코드보다 테스트 코드가 위주가 될 수 없다.
테스트가 쉬워지는 방향보다는, 프로덕션 코드가 더 쉽게 만들어지도록 하는 것이 우선!
 
범용적일 필요가 없다. 코드 중복을 줄이는 건 좋지만, 확장성은 객체가 여러 곳에서 쓰인다는 것과 동일할 수도 있다.
객체는 특정 역할만 수행하면 된다. 결합도가 높아지는 것은 좋지 않다.

리뷰어님의 말씀 중에 가장 인상깊은 부분을 가져왔다.
이번 미션을 진행하면서 객체지향에 한 발짝 다가간 느낌이다.
 

---

💬 프로그램 실행 종료의 로직

   private void printGameResultWithRetry(final Participants participants, final List<String> ladderResultNames) {
        List<Participant> resultParticipants;
        do {
            resultParticipants = getResultParticipants(participants);
            printGameResult(resultParticipants, ladderResultNames);
        } while (isProceed(resultParticipants));
    }


    private List<Participant> getResultParticipants(final Participants participants) {
        return inputView.getInputWithRetry(() -> {
            String resultParticipantName = inputView.getResultParticipantName();
            return getParticipants(participants, resultParticipantName);
        });
    }


    private List<Participant> getParticipants(final Participants participants, final String resultParticipantName) {
        if (resultParticipantName.equalsIgnoreCase(EXIT_INPUT_MESSAGE.getMessage())) {
            return null;
        }
        return participants.getResultParticipants(resultParticipantName);
    }


    private boolean isProceed(final List<Participant> resultParticipants) {
        return resultParticipants != null;
    }


    private void printGameResult(final List<Participant> resultParticipants,
    final List<String> ladderResultNames) {
        if (resultParticipants == null) {
            return;
        }
        OutputView.print(System.lineSeparator() + RESULT_GAME_GUIDE.getMessage());
        outputView.printLadderGameResult(resultParticipants, ladderResultNames);
    }
}

코드가 길어서 알아보기 힘들겠지만...!
EXIT를 입력했을 때 프로그램이 종료되도록 만든 부분이다.
다만, 종료 조건으로 getParticipants()에서 명시적으로 null을 반환하도록 만든 것인데 (사용자가 입력한 console 메시지의 String 값은 null이 들어올 수 없기 때문에 Exit를 입력했을 경우 사용자 목록에 대해 null을 반환하도록 처리) 내가 봐도 이 부분은 이상했다. ㅎㅎ
 
그래서 이에 대해 리뷰어님이 다음과 같은 피드백을 주셨다.

리뷰를 듣고 머리가 띵했다. 결국 null을 받게 된 이유는, List 자체로는 표현할 수 없는 개념이 존재했기 때문이다.
객체의 책임은 어디에서 오는가?로 생각해봤을 때, 필요한 기능들에 대해 수행하는 역할을 하는 것이겠구나...! 어떠한 기능을 수행하는 역할을 위해 객체를 생성할 수 있겠구나! 라고 깨달을 수 있었다.
 
그래서 다음 피드백에는 DTO를 활용해주었다.

public class ResultParticipantsDto {

    private final boolean isProceed;
    private final List<Participant> participants;

    public ResultParticipantsDto(final List<Participant> participants, final boolean isProceed) {
        this.participants = participants;
        this.isProceed = isProceed;
    }

    public static ResultParticipantsDto create(final List<Participant> participants, final boolean isProceed) {
        return new ResultParticipantsDto(participants, isProceed);
    }

    public boolean isProceed() {
        return isProceed;
    }

    public List<Participant> getParticipants() {
        return List.copyOf(participants);
    }

다음과 같이 현재 진행 중인 상태에 대해서도 값을 가지고 있고, 참가자의 리스트에 대해서도 가진 결과 DTO를 활용하는 것이다.

    private void printGameResultWithRetry(final Participants participants, final List<String> ladderResultNames) {
        ResultParticipantsDto resultParticipantsDto;
        do {
            resultParticipantsDto = getResultParticipantsDto(participants);
            outputView.printLadderGameResult(resultParticipantsDto, ladderResultNames);
        } while (resultParticipantsDto.isProceed());
    }

    private ResultParticipantsDto getResultParticipantsDto(final Participants participants) {
        return inputView.getInputWithRetry(() -> {
            String resultParticipantName = inputView.getResultParticipantName();
            return makeResultParticipantsDto(participants, resultParticipantName);
        });
    }

    private ResultParticipantsDto makeResultParticipantsDto(final Participants participants, final String resultParticipantName) {
        if (resultParticipantName.equalsIgnoreCase(EXIT_INPUT_MESSAGE.getMessage())) {
            return ResultParticipantsDto.create(Collections.emptyList(), false);
        }
        List<Participant> resultParticipants = participants.getResultParticipants(resultParticipantName);
        return ResultParticipantsDto.create(resultParticipants, true);
    }
}

덕분에 null을 반환하지 않고, isProceed에 대한 false / true, 그리고 Collections.emptyList() 를 사용하면서 더 용이하게 처리할 수 있었다. NPE는 피하자...! 이래서 코틀린을 많이 선호하는구나...도 배울 수 있던 👍

---

 
사실 이것 외에도 되게 많은 것들을 배웠지만...!
PR 커멘트에서 충분히 드러난 것들이기 때문에 이정도로 작성해두겠다.
사다리 미션은 리뷰어님이 너무너무너무 좋았어서 레벨 1 미션 중 가장 행복했다 :D
얼른 다른 미션들도 회고 글을 작성해야겠다 ㅎㅎ;;