1주차 마무리 : 코드 리뷰!
1주차를 마무리하며, 다른 지원자분들과 함께 코드 리뷰를 주고받았습니다. 지난 프리코스에서는 구현 자체가 힘들어서 주차 시작되자마자 미션에 매달리느라 (그리고 리뷰 받기에는 코드가 너무 허접하다고 생각해서) 참여하지 않고 넘어갔었는데요, 이번에는 구현 시간에 좀 여유를 느끼기도 하고, 남의 시선에서 내 코드를 보는 것과 남이 작성한 코드를 읽어보고, 이해하고, 개선점을 찾아보는 경험이 얼마나 중요한지 알기에 적극적으로 참여했습니다.
리뷰 해본 후기 : 시야의 확장
다양한 방식의 구현을 볼 수 있어서 많이 배웠습니다. 제가 포기했던 재귀 구조로 멋지게 구현하신 분도 있었고, MVC 패턴이 필요할 만큼 복잡한 구조가 아니라고 판단해서 단순하게 분리하신 분도 있었구요.
저는 거의 관성적으로 MVC를 적용하려고 했는데, 다른 코드들을 보면서 혹시 내가 필요성을 고려하지 않고 어울리지 않는 부분에 무리하게 MVC 패턴을 적용하려고 하고 있는 건 아닌가 하는 생각이 들었습니다. 다만 저로서는 이 방법이 내가 이 앱의 구조를 이해하기에는 가장 좋은 방법이라 생각하는데, 코드를 읽는 다른 분들의 입장에서는 어떨지 모르겠네요...
그리고 열심히 코드 리뷰를 달아주신 분들의 PR에 답 리뷰를 달면서, '이 분도 내 코드 열심히 봐주셨으니까 나도 열심히 봐야지!'하고 열의를 갖고 임하게 되더라구요. 덕분에 이 앱이 어떤 구조로 구성되어 있는지, 불필요하거나 잘못된 부분은 없는지 꼼꼼하게 보게 되어 더 양질의 리뷰가 가능했던 것 같습니다. 좋은 경험이었어요!
리뷰 받아본 후기 : 사고의 확장
등에 뭐 묻은 걸 알려주는 느낌이라고 해야 할까요? 미처 생각하지 못했거나, 별 고민을 안했거나, 아예 몰랐던 부분을 잡아주셔서 많이 배울 수 있었습니다.
의존성 주입
어떤 클래스나 모듈이 다른 클래스나 모듈의 기능을 사용할 때 이를 의존성이라고 하는데, 객체 지향 프로그래밍에서는 객체가 의존성을 직접 생성하거나 찾는 대신 외부에서 의존성을 주입하도록 하여 모듈성, 테스트 용이성, 유연성을 더 확보할 수 있습니다.
기존의 코드에서는 BaseballGameController가 InputView, OutputView, BaseballGame인스턴스를 직접 생성하고 있었는데, 이를 App에서 생성하고 컨트롤러에 파라미터로 넘겨주는 식으로 의존성 주입을 적용해보았습니다!
다만, 현재는 유틸 함수로 빼 놓은 함수들(validateUtils, viewUtils, gameUtils)도 따지고 보면 외부의 구현이고, 이를 직접 불러와서 사용하는 것도 일종의 의존인데, 이것들도 다 주입해야 하는 건가? 애초에 분리를 하면 안됐던 건가? 하는 새로운 고민이 생겼습니다...🤔
불필요한 파라미터 전달, 드릴링 문제
컨트롤러 내에 #baseballGame 필드를 만들어놓고 그 안에 BaseballGame 인스턴스를 확장해놨는데도, 불필요하게 game을 함수의 파라미터로 넘겨주고 있는 부분을 지적해주셨어요!
게다가 readNumbersInput은 단순히 processNumbers 메서드의 파라미터에 넘겨주기 위한 목적으로 game 파라미터를 받고 있는데, 이는 리액트에서 말하는 props drilling처럼 부자연스러운 맥락이 될 것 같다는 조언도 남겨주셨습니다. game 파라미터를 받아 활용하는 부분을 this.#baseballGame 필드를 직접 불러오게끔 수정하여 해결했습니다!
MissionUtils 통째로 불러오는 거
MissionUtils를 잘게 쪼게서 필요한 함수만 불러오도록 만들었다가 테스트에 실패했던 적이 있었는데요, 단순히 테스트를 통과하는지 확인하려고 모든 경우에 MissionUtils를 통째로 가져오도록 바꿔놓고 그대로 제출했습니다.
이후 MissionUtils를 통째로 불러오기보단 필요한 것만 부분적으로 (Console, Random) 불러오면 더 좋을 것 같다 는 리뷰를 받았고, 그런 식으로 바꿔봤는데 잘 작동하더라구요!
테스트를 통과한다고 그냥 넘기지 말고, 기존과 다른 방법으로 개선할 방안이 없는지 조금만 더 고민해봤으면 좋았겠다고 생각했습니다😅
함수의 책임 분리
나름 열심히 고민했다고 생각했는데, 결과문자열을 생성한는 함수에서 낫싱일 때, 3스트라이크일 때, 일반적인 경우일 때 각각 알맞는 메세지를 반환하는 로직과 결과 문자열을 생성하는 로직을 동시에 갖고 있는 점을 짚어주셨습니다.
function getResultMessage(matchResult) {
const [ball, strike] = matchResult;
if (!ball && !strike) {
return GAME_RESULTS.noMatch;
}
const resultString = getResultString(ball, strike);
if (strike === GAME_CONSTANTS.strikeOutCount) {
return `${resultString}\n${GAME_MESSAGES.finish}`;
}
return resultString;
}
function getResultString(ball, strike) {
const text = [GAME_RESULTS.ball, GAME_RESULTS.strike];
const parsedResults = [ball, strike].map((item, idx) => {
if (!item) return;
return item.toString() + text[idx];
});
return parsedResults.join(' ').trim();
}그래서 이런 식으로! 결과가 낫싱이면 noMatch를 반환하고, 아닐 경우 getResultString으로 분리된 결과문자열 생성 함수로 문자열을 만든 후에, 3스트라이크이면 finish메세지와 묶어서, 아니라면 그냥 resultString만 반환하는 식으로 분리해주었습니다! 이제 조건에 맞는 결과 메세지를 반환하는 함수와 볼, 스트라이크 개수 문자열을 만들어주는 함수가 분리되었네요!!
InputView, OutputView는 왜 class로 만드셨나요?
제일 고민이 많았던 부분을 짚어주셨습니다...😅 InputView, OutputView는 내부적으로만 사용하는 메서드도 없고, 가지고 있는 내부 상태도 없는데 왜 클래스여야 할까요?
제 나름의 결론은 이렇습니다 :
- View가 단순 객체로 전역에서 접근 가능하게 되면, 어느 위치에서든 직접 수정이 가능하다. 이렇게 되면 예기치 않은 사이드 이펙트가 발생할 수 있다. 클래스로 View를 정의하면, 명확한 인터페이스를 통해만 접근하게 되므로 이러한 위험성을 줄일 수 있다.
이 이외에는 마땅한 근거를 찾을 수가 없더라구요...🥲 근데 그렇다면 정말 InputView는 클래스여야 하는 걸까요? 계속 고민해봐야 할 것 같습니다...
