[2단계 - 사다리 게임] 미아(이종미) 미션 제출합니다.

[jongmee]

안녕하세요 스티치! 1단계 피드백 반영과 2단계 기능 구현을 진행하였습니다 😊

피드백 반영

1. LadderGameManager 객체 삭제
   의존성 주입이 필요치 않고, 하나의 객체만 사용하도록 하는 것은 LadderGame 내부에서도 할 수 있기에 리뷰 주신 대로 수정하였습니다!
2. copyOf 메서드 대체
   해당 메서드의 내부 구현을 확인해보니, HashSet을 생성하고 Set.of 를 호출하면서 불필요하게 메모리를 더 쓰고 있다는 것을 알 수 있었습니다. 따라서 HashSet을 생성하는 것으로 대체하였습니다.
3. Custom Exception 삭제
   따로 예외 처리를 하는 로직이 존재하지 않고, 프로그램 규모를 고려했을 때 꼭 필요하다고 생각하지 않아 삭제하였습니다.
4. Path 도메인 클래스의 출력용 속성을 OutputView에서 관리
   shape라는 속성은 단순 출력할 때만 쓰이기 때문에 객체의 영역에 맞게 OutputView의 상수로 분리하였습니다.

2단계 미션 중 고민되었던 점

1. position (int)을 포장해야 하는가?
   제가 판단하기에, 필요한 상황에서만 primitive type을 포장하는 것이지 반드시 포장해야 되는 건 아니라고 생각합니다. 불변 객체의 특징을 활용하지 않아 아래 Participant 객체의 속성을 불변 객체 Position에서 int type으로 변경하였습니다.

public class Participant {
    private final Name name;
    private int position;

position은 사다리 타기 게임의 참가자의 사다리상 위치(첫 시작 위치, 최종 위치)를 저장합니다. 스티치는 이 상황에서 따로 Position 객체를 생성해서 얻을 수 있는 장점이 있다고 생각하시나요? 스티치의 의견이 궁금합니다!

2. position을 신뢰성 있게 관리하는 방법이 무엇일까?
   역시 position에 관한 부분인데요 😂 position에 최종 위치를 저장하게 되면서 아래와 같이 값이 변경되는데, 메서드명으로 목적만 명시 했을 뿐이지 setter와 다름없습니다. 외부에서 해당 메서드를 호출하여 값을 조작할 수 있게 됩니다. 안전하게 position을 관리하는 방법에 대해 힌트를 얻고 싶습니다.

    public void setFinalPosition(final int finalPosition) {
        this.position = finalPosition;
    }

이번 리뷰도 잘 부탁드립니다! 🙇🏻‍♀️

[lxxjn0]

미아, 2단계 구현하시느라 수고 많으셨습니다.

복잡한 요구사항일 수 있는데 1단계에서 적절하게 잘 분리해두신 것 같아서 이번 단계에는 구조에 대한 큰 어려움이 없었던 것 같습니다 👍🏼

몇 가지 부분들에 대해 피드백 남겨놨는데 한번 확인 부탁드리겠습니다. 추가로 남겨주신 답변 중 두 번째 질문에 대한 답변은 피드백에 포함되어 있고 첫 번째 질문에 대한 답변을 드리자면 미아의 판단이 적절할 것 같습니다. 단순히 원시값을 감싸야해서 감싸기보다는 미아의 판단에 굳이 감싸서 관리할 값이 아니라면 감쌀 필요가 없는게 맞아보입니다. 제가 생각하기에도 해당 값은 비즈니스적인 의미라기 보다는 코드의 로직 진행을 위해 필요한 값이므로 굳이 객체로 관리해서 얻을 수 있는 장점은 크지 않을 것 같습니다!

[lxxjn0]

[제안] 가독성이 좋은 코드를 작성하는 방법 중 하나는 코드의 흐름에 맞게 배치를 하는 것도 포함될 수 있습니다. 코드가 읽히는 순서대로 메서드와 변수 등을 배치한다면 더 가독성이 좋은 코드를 작성하실 수 있을 것 같아요 :)

[jongmee]

    public void run() {
        final Participants participants = exceptionHandler.retryOnException(this::createParticipants);
        final LadderGamePrize ladderGamePrize = exceptionHandler.retryOnException(this::readLadderGameResult);
        final int width = participants.getNecessaryLadderWidth();
        final Ladder ladder = createLadder(width);
        participants.playAll(ladder);
        final GameResults gameResults = ladderGamePrize.calculdateGameResults(participants);

        printLadder(participants, ladder, ladderGamePrize);
        printGameResult(gameResults);

        inputView.closeResource();
    }

이렇게 변경해보았는데 읽기 편하실까요..?! 출력 메서드를 마지막에 모아주었습니다.

[lxxjn0]

앗.. 제가 리뷰를 헛갈리게 드렸네요 ㅠㅠ run 메서드가 참조하는 메서드들을 흐름에 맞게 나열하면 좋을 것 같다는 이야기였습니다!! 👍🏼

[jongmee]

메서드 순서도 가독성에 아주 중요하죠! 놓친 부분 알려주셔서 감사합니다 😊

[lxxjn0]

[제안] 이번 기회에 Optional이 가지는 장점과 적절한 활용 방안에 대해 한번 학습해보시는 것도 좋을 것 같아요!

[jongmee]

Optional을 null 체크 용으로 사용하는게 생성 비용 대비 비효율적이군요. 함수를 분리하고 함수명에 left, right 키워드를 사용해서 가독성을 높이고 싶었는데 되려 독이 됐네요 😢

---

이전 코드

    public int findNextParticipantPosition(final int participantPosition) {
        Optional<Integer> leftPathPosition = getLeftPosition(participantPosition);
        if (leftPathPosition.isPresent()) {
            return leftPathPosition.get();
        }
        Optional<Integer> rightPathPosition = getRightPosition(participantPosition);
        return rightPathPosition.orElse(participantPosition);
    }

리팩토링된 코드

    public int findNextPosition(final int position) {
        if (position > 0 && isExistIn(position - 1)) {
            return position - 1;
        } else if (position < ladderPaths.size() && isExistIn(position)) {
            return position + 1;
        }
        return position;
    }

저는 오히려 리팩토링하고 가독성이 더 괜찮아진 거 같은데, 스티치 생각은 어떠신가요?

[lxxjn0]

맞습니다!! Optional은 비용이 많이 드는 객체이지만 그 만큼의 의미를 가질 수 있는 상황에서 사용한다면 적절한 도구라고 생각합니다. 지금처럼 한 메서드 내에서는 굳이 사용할 필요가 없어서 피드백 드렸어요. 저도 기존의 코드보다는 바뀐 코드가 더 가독성이 좋다고 생각합니다. 조건문을 따로 메서드로 뽑으면 코드를 읽는 사람에게 조건의 의미를 더 쉽게 전달할 수 있을 것 같다고 생각합니다 :)

[lxxjn0]

[질문] 해당 클래스를 일급 컬렉션으로 만드신 이유가 무엇일까요?

[jongmee]

현재는 private final Map<String, String> values;로 변경되었는데요.

Collection의 불변성을 보장하여 외부에서 조작되는 것을 막고 사다리 게임 결과의 기능(findByName)과 값을 GameResults 클래스 한 곳에서 관리하기 위함이었습니다!

[lxxjn0]

단지 컬렉션을 감싸기만 한 객체라는 느낌이 들었습니다. 감싼 컬렉션으로 유의미한 관리를 하거나 특별한 로직을 수행하지 않는다면 굳이 일급 컬렉션으로 관리할 필요가 없지 않을까 생각합니다.

[jongmee]

어떤 기능을 도입할 때 해당 기능이 사용되고 있는지(정말 필요한지) 생각하며 프로그래밍하는게 중요하다는 걸 여러 번 깨닫고 갑니다 👍

[jongmee]

안녕하세요 스티치! 피드백 반영하여 리팩토링하고 몇 리뷰에 코멘트 남겨 놓았습니다.

또한 질문이 있습니다..!
제 PR 질문에 대한 스티치의 답변 중에서

제가 생각하기에도 해당 값(position)은 비즈니스적인 의미라기 보다는 코드의 로직 진행을 위해 필요한 값이므로 굳이 객체로 관리해서 얻을 수 있는 장점은 크지 않을 것 같습니다!

라는 부분이 있었는데요. 비즈니스적인 의미와 코드의 로직 진행을 위해 필요한 값의 차이를 모르겠어서 구체적인 예시를 들어주실 수 있을까요?

다시 한 번 리뷰 요청 드려요!

[lxxjn0]

미아, 피드백 반영하시느라 수고하셨습니다 👍🏼

앞선 피드백들을 꼼꼼하게 잘 반영해주셔서 추가적으로 피드백 드릴만 한 부분이 없는 것 같습니다. 전체적으로 코드가 잘 정리된 것 같아요!! 사다리 게임 미션이 복잡한데도 멋지게 구현해주셨네요. 미션 진행하시느라 수고 많으셨습니다 :)

그리고 남겨주신 질문에 대한 대답을 드리자면 비즈니스적인 의미는 프로그램의 요구사항이라고 보시면 될 것 같아요. 약간은 개발자 개인의 판단 범위에 따라 달라질 수 있는 부분이라고는 생각합니다.

이전에 position에 대해서 이런 피드백을 드렸던 것 같은데, 예시로 참가자의 이름은 실제 요구사항에 등장하는 도메인이라고 생각합니다. 참가자의 이름이 5자 단위로 출력이 가능해야 하는데, 이를 통해 개발자는 참가자의 이름 길이를 5자로 제한하였습니다. 이때의 '참가자의 이름'은 요구사항에 등장한 정보이고 그렇기에 '비즈니스적인 의미'를 지닌다고 생각합니다.

그러나 미아가 만든 position이라는 값은 실제 미션의 요구사항에 등장하지 않은 내용입니다. 참가자와 상품을 매칭시킬 수 있는 방법은 여러 방법이 있을 수 있지만 참가자나 상품의 등장 순서를 위치라는 속성으로 정의하고 이를 게임 결과를 얻는 수단 중 하나로 사용하였습니다. 실제 미션의 요구사항에는 존재하지 않았지만 '게임을 진행'하기 위한 수단으로 개발자가 정의한 정보이고 이러한 유형의 정보를 '코드 진행을 위해 필요한 값'이라고 제가 말씀드렸던 것 같습니다.

즉, 실제 비즈니스가 관심을 가지는 도메인의 영역 중 핵심적인 부분이 아닌, 개발자가 프로그램의 수행과 진행을 위해 설정한 부분들은 개발자의 자율의 영역에 가깝다고 생각했고 그래서 그러한 피드백을 전달드렸던 것 같습니다!

제가 전달하고자 하는 내용이 잘 전달되었을 지는 모르겠네요 ㅎㅎ.. 혹시 잘 이해가 안가신다거나 더 궁금한 내용이 있다면 언제든 DM으로 연락주셔도 됩니다!!

미션 진행하시느라 수고하셨습니다 👍🏼

[lxxjn0]

앗.. 제가 리뷰를 헛갈리게 드렸네요 ㅠㅠ run 메서드가 참조하는 메서드들을 흐름에 맞게 나열하면 좋을 것 같다는 이야기였습니다!! 👍🏼

[lxxjn0]

맞습니다!! Optional은 비용이 많이 드는 객체이지만 그 만큼의 의미를 가질 수 있는 상황에서 사용한다면 적절한 도구라고 생각합니다. 지금처럼 한 메서드 내에서는 굳이 사용할 필요가 없어서 피드백 드렸어요. 저도 기존의 코드보다는 바뀐 코드가 더 가독성이 좋다고 생각합니다. 조건문을 따로 메서드로 뽑으면 코드를 읽는 사람에게 조건의 의미를 더 쉽게 전달할 수 있을 것 같다고 생각합니다 :)

[lxxjn0]

단지 컬렉션을 감싸기만 한 객체라는 느낌이 들었습니다. 감싼 컬렉션으로 유의미한 관리를 하거나 특별한 로직을 수행하지 않는다면 굳이 일급 컬렉션으로 관리할 필요가 없지 않을까 생각합니다.

[jongmee]

스티치의 대답은 잘 이해되었습니다! 감사합니다.

이번 미션의 코드 리뷰를 통해서 불필요한(혹은 불필요할 수 있는) 코드와 기능을 피드백 받고, 코드의 개선 방향을 더 고민할 수 있었습니다. '왜 필요한가?', '장점은 무엇인가?' 등의 질문을 던지며 많이 성장할 수 있었습니다. 저 역시 프로그램의 요구사항을 만족시키는 선에서 프로그램의 크기는 작을 수록 좋다고 생각하기 때문에 많이 배워갑니다 👍

소중한 리뷰 감사합니다!