우아한테크코스 LV 1 - 사다리 타기 피드백 정리

0. 들어가기 전

사다리 타기는 오늘 미션이 끝나는데

자동차 경주 미션 피드백 정리와는 달리 끝나는 날인 오늘 정리해보려고 한다!

 

이렇게 바로 정리하는 것도 내일 바로 새로운 미션이 시작되기 때문에

다음 미션에 바로 적용해볼 수 있어서 괜찮을 것 같다!

---

1. 리뷰어 피드백

 

🎯 1) static 사용 관련

static 사용 관련해서 언제 사용해도 되는건지 기준이 모호해서 질문을 드렸었다.

 

리뷰어님께선 먼저 '해당 클래스를 여러 객체로 생성할 필요가 있는가?'를 고민해본다고 하셨다.

'상태를 지니는가?'도 고민해보고 상태를 지니면 공통된 객체가 아니라,

인스턴스화된 객체로 여러 객체로 생성해야하므로 static을 쓰지 않는다고 하셨다.

 

이렇게 상태를 가지고 있지 않는 클래스 '유틸리티 클래스'라고 한다.

 

내 코드에서는 View에서 static을 사용했었는데,

View는 상태를 가지고 있지 않고 항상 똑같은 메소드를 호출하기만 해서

사용해도 괜찮다고 판단하였다.

 

앞으로, 상태를 가지고 있지 않는 클래스에서는 static 도입을 고려해보고,

상태를 가진 클래스라면 static을 사용하지 않고 인스턴스화를 시켜야겠다고 판단 기준을 세웠다!

 

🎯 2) 생성자에서 모든 필드를 초기화 시켜줘야 하는지

처음에 클래스가 필드를 가지고 있을 때,

해당 필드의 초기화를 해당 클래스가 생성될 시점에 할지,

일단 생성을 해놓고 그 후에 메소드로 초기화를 시켜줄지 고민하고

어떤 방식이 나을지 리뷰어님께 질문을 드렸다.

 

답변으로 온 리뷰어님의 첫 문장은 다음과 같았다.

'객체가 생성되어 사용될 때는 이미 완전한 상태로 사용되어야 합니다!'

 

 

🎯 3) 불필요한 의존성 주입 제거

PR을 보냈던 내 코드에서 불필요한 의존성 주입이 존재했다.

 

LadderGame에서 BooleanGenerator의 구현체인 RandomBooleanGenerator를 생성해서
LadderGame -> LadderMaker -> Line 순으로 Line에게 넘겨주는데,

 

LadderGame에서 생성해서 LadderGame -> LadderMaker로 넘겨주지 말고,
LadderMaker에서 생성해서 Line에 넘겨주는 것이 좋지 않을까?

 

현재 LadderMaker 입장에서는 단순히 Line에게 RandomBooleanGenerator를 넘겨주는데
의미없는 코드가 되는 것이 아닌가?

 

결국 BooleanGenerator 인터페이스를 LadderGame, LadderMaker, Line이 의존하는데,
LadderMaker에서 생성해서 Line에 넘겨주면 LadderGame은 의존이 없어져서
의존성을 하나 없애는게 되므로 좋지 않을까?

 

이러한 생각을 거쳐서 LadderGame -> LadderMaker로 의존성을 끊어주고

LadderMaker에서 BooleanGenerator를 가지고 Line에게 넘겨주었다.

 

앞으로는, 어떤 객체가 단순히 받은 객체를 전달만 하는 것은 아닌지 의식적으로 생각해야겠다!

 

 

🎯 4) Map 자료구조 네이밍

이전 자동차 경주에서도 살짝 언급이 있었는데,

Map 자료구조를 'Map'이라는 네이밍을 안 쓰고 어떻게 할 수 있는지 여쭤봤다.

 

답변은 리뷰어님이 아니라 ChatGPT에게 왔다 ㅋㅋㅋㅋ

리뷰어님이 ChatGPT에게 물어본 사진을 캡쳐해서 보내주셨다.

이렇게 여러 네이밍을 추천해줬는데, 리뷰어님 개인적으로는 'By'를 선호하지 않는다고 하셨다.

하지만 나는 '-s'를 쓰면 리스트 자료구조와 헷갈릴 것 같아서,

앞으로도 'By'를 써서 네이밍을 할 것 같다~!

 

🎯 4) TDD 시 테스트 작성 기준

이번 미션 요구사항 중에 TDD로 진행하기 때문에 '모든 기능의 단위테스트가 존재해야한다.'가 있었다.

 

나는 이러한 요구사항에 따라 단순히 클래스 생성 테스트와, Getter 테스트 등

약간 테스트할 필요 없는? 기능들도 테스트를 진행하게 되었다.

 

진행하면서도 필요없는 테스트같다고 느껴서 리뷰어님에게 TDD 시에 이런 테스트가 필요한지 여쭤봤다.

 

리뷰어님은 '필수'는 아니라고 생각한다고 하셨다.

 

우선 Getter 테스트 같은 경우에는

따로 테스트하지 않아도 사용하는 기능들에서 자연스럽게 테스트가 된다고 하셨다.

 

이러한 의미에서 클래스 생성 테스트도

결국 다른 테스트를 수행할 때 자연스럽게 테스트가 되므로 굳이 할 필요가 없다고 답변을 주셨다.

 

앞으로는 의미없다고 판단이 되는 테스트는 지양하도록 노력해야겠다!

 

🎯 5) Controller의 객체지향적 구조

나는 Controller를 설계할 때 '가독성'을 위주로 설계했었다.

Controller는 게임의 흐름을 제어하는 역할이라고 생각해서

한 눈에 봤을 때 게임의 흐름이 읽혀야한다고 생각했고, 이를 메소드 네이밍으로 풀어냈다.

프리코스부터 거의 모든 미션을 다음과 같은 구조로 설계해서 제출했었다.

public class LadderController {

    public void start() {
        generateLadderGameStep();
        printGameResultStep();
        printResultByPlayerStep();
    }
}

이렇게 외부에서 컨트롤러의 start()를 호출하면,

여러 단계로 나뉘어서 'Step'이라는 메소드 네이밍으로 어떤 흐름을 가지는지 파악하도록 설계했다.

 

이 구조를 리뷰어님이 보고 하신 말씀은 '객체지향적' 구조가 아닌 것 같다고 말씀하셨다.

 

현재 구조는 메소드의 파라미터와 반환 값이 없이 함수 호출만 있는데,

이러한 구조는 객체지향적이 아니라고 하셨다.

 

객체지향적인 구조란, 객체 간에 메시지를 주고 받는 구조라고 하셨다.

따라서, generateLadderGame()을 호출하면, LadderGame을 반환받고 

해당 LadderGame을 다음 Step에 사용하는 그런 구조로 리팩토링해보라고 하셨다.

 

해당 구조 리팩토링은 시간 부족으로 인해서 잘하지는 못했지만,

리뷰어님이 말씀하시는 구조가 어떤 구조인지 이해할 수 있었다.

 

앞으로 컨트롤러 단 코드를 작성할 때 '객체지향적' 구조로 설계하도록 노력해야겠다!

 

🎯 6) 포장 클래스의 Getter를 포장 클래스를 가지고 있는 객체에서 사용해도 될까?

포장 클래스를 View에 전달하기 위해서 Model 객체가 가진 포장 값을 풀어서 전달하려고 했다.

이때, 포장 값을 풀기 위해서 Model 객체 내부에서 포장 클래스의 getter를 호출해야 했었다.

 

Player가 가지고 있는 Name을 Player가 Name.getRawName()으로 풀어서 전달하려고 했다.

public class Player {

    private final Name name;

    public String getName() {
        return name.getRawName();
    }
}

 

처음에 든 생각은, Name도 포장 클래스이긴 하지만 Model 객체인데

다른 Model 객체인 Player가 다른 Model 객체의 getter를 호출하는 것이 아닌가?

라는 생각이 들었다.

 

이 부분이 궁금해서 리뷰어님께 질문을 드렸다.

리뷰어님은 다음과 같은 답변을 주셨다.

* Name은 Player의 이름을 위해 만들어진 값 객체이다.
* 따라서 Player에서 Name을 getter로 호출할 때 값 객체의 포장을 푸는 것이 이상하지 않다.
* 오히려 외부에서 Name의 getter를 호출해야하는 번거로움을 줄일 수 있다.

Player가 이미 Name을 가지고 있고 의존하고 있어서

그 안에서 getter를 사용하는 것이 이상하지 않다고 생각해주신 것 같다!

 

앞으로 원시 값을 포장하는 일이 많을 것 같은데, 이러한 근거를 가지고 풀어서 넘기면 될 것 같다!

 

🎯 7) 객체의 생성자에서 너무 많은 일이 일어날 때?

내 코드에서 LadderGame을 생성할 때, LadderGame의 생성자에서 많은 일을 수행했었다.

public class LadderGame {

    public LadderGame(Players players, Results results, Ladder ladder, ResultByPlayer resultByPlayer) {
            this.players = players;
            this.results = results;
            validateResultsSize();
            LadderMaker ladderMaker = new LadderMaker(new LadderProperty(names.size() - 1, height));

            this.ladder = ladderMaker.generate();
            this.resultByPlayer = generateResultByPlayer();
        }
}

검증 로직과 더불어서 ladder와 resultByPlayer를 초기화 해주는 역할을

LadderGame 안의 메소드를 호출해서 해주고 있었다.

 

이 부분을 리뷰어님께서 외부에서 생성해서 LadderGame에 넣어주는 게 좋다고 하셔서

LadderGame을 만들어주는 LadderGameMaker를 만들어 리팩토링하게 되었다.

public class LadderGameMaker {

    public static LadderGame createLadderGame(List<String> names, List<String> results, int height) {
        Players generatedPlayers = new Players(generatePlayers(names));
        Results generatedResults = new Results(generateResults(results));
        LadderMaker ladderMaker = new LadderMaker(new LadderProperty(names.size() - 1, height));
        Ladder generatedLadder = ladderMaker.generate();
        ResultByPlayer generatedResultByPlayer = generateResultByPlayer(generatedPlayers, generatedLadder, generatedResults);

        return new LadderGame(generatedPlayers, generatedResults, generatedLadder, generatedResultByPlayer);
    }
}

public class LadderGame {

    public LadderGame(Players players, Results results, Ladder ladder, ResultByPlayer resultByPlayer) {
        this.players = players;
        this.results = results;
        validateResultsSize();
        this.ladder = ladder;
        this.resultByPlayer = resultByPlayer;
    }
}

이렇게 하니 LadderGame이 더 안전해진 것 같았다.

대신 파라미터가 4개가 되어서 찝찝했는데,, 시간도 부족했고 어떻게 해결할지 모르겠어서 여기서 멈췄다.

---