-
Notifications
You must be signed in to change notification settings - Fork 8
[차세진] 자동차 경주미션 Step2 #13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: chasj0326
Are you sure you want to change the base?
Conversation
suyeon1218
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요 세진님...!! 코드 잘 봤습니다...! 세진님께서 구현에 힘을 주셨다는 얘기를 듣고 저도 코드 자체에 에러가 있는지 보다는 세진님이 어떻게 구현했는지를 파악하려고 노력하면서 우와 우와< 같은 리뷰 위주로 남기게 된 것 같아요... ㅋㅋㅋ
노션 클로닝을 할 때 막히는 부분이 있으면 세진님 코드를 참고하며 놀랐던 부분이 이만저만이 아니었는데 이번 미션도 역시 세진님의 문제해결능력이 돋보이는 코드였습니다...!! 세진님의 코드를 보면 더 시야가 넓어지는 기분이라 꼭 참고하려고 하고 저도 더 열심히 하게 되는 것 같아요. 흑흑... 제 코드 리뷰가 뭔가 인사이트를 얻으실 수 있는 리뷰라기 보단 세진님의 코드를 보며 감탄 & 궁금한 점만 적은 코드 같아 부끄럽네요 ㅠ_ㅠ 아무튼 이번주 미션도 수고 많으셨습니다...!!
| DiceDiffGame, | ||
| }, | ||
| }), | ||
| viewer: new RacingGameViewer(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저는 main 을 컨트롤러의 역할로 분리했는데 세진님 처럼 Controller 를 따로 만들고, 컨트롤러에 View 를 전달해주는 방식이 가독성 측면이나 재활용 측면에서 좋은 것 같아요! 고민을 많이 하신 흔적이 보여서 세진님의 코드를 보는 게 매번 기대가 됩니다 ㅎㅎ
| get results() { | ||
| return this.#results; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
results 가 배열이라서 복사등을 사용하지 않으면 밖에서 데이터를 조작할 수 있지 않을까요?!
| get miniGames() { | ||
| return this.#miniGames; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
마찬가지로 객체 형태이기 때문에 밖에서 조작할 수 있을 것 같아요!!
| this.name = name.replaceAll(' ', ''); | ||
|
|
||
| this.validate() | ||
| this.#position = 0 | ||
| this.validate(); | ||
| this.#position = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
name private 속성이 아니니까 밖에서 접근하고 변경할 수 있을 것 같아요...! 그럼 이 경우 name 의 유효성 검사는 어디서 진행해줄 수 있나요?
| name => positions[name] === this.maxPosition, | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maxPosition 정보가 필요해서 이걸 프로퍼티로 저장해서 관리할지 고민하다가 그냥 저는 하나의 함수안에 합쳐버렸는데 분리해놓으니 훨씬 더 읽기 편해지네요...! 인스턴스로 관리하는 변수만 get 프로퍼티를 사용할 수 있다고 무의식적으로 생각했는데 세진님 덕에 고정관념을 깨고 갑니다 😀
| this.cars = []; | ||
| this.players = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.cars 와 this.players 를 여기서 처음으로 초기화시켜주는군요...! 사용되는 프로퍼티를 위에 미리 선언해 놓지 않은 이유가 있으실까요?
| this.players.push(newCar.name); | ||
| } | ||
| }); | ||
| this.validate(this.cars, ['leastCarCount', 'uniqueCarName']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기서 자동차의 이름에 대한 유효성 검사가 진행되기 때문에 Car 클래스에선 따로 유효성 검사를 안 해주는 거군요...! 세진님은 유효성 검사가 어느 수준만큼 이뤄져야 한다고 생각하시나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이런식으로 validator 를 만들수도 있군요...!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
객체 다루기 마스터!
seeyoujeong
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 잘봤습니다!
세진님 코드는 항상 흥미롭습니다 ㅎㅎ
이번엔 미니게임에 진심을 다하신 모습 보기좋았어요 ㅋㅋㅋ
템플릿이나 유효성 검사하는 코드에서 많이 배워갑니다!
- 이벤트를 활용하는 방식이 뷰와 로직을 분리한다는 관점에서 유의미한 방법인지 궁금합니다 !
저도 잘몰라서 유의미한 방법인지는 모르겠지만 관리하기 편해보여서 좋았어요!
| if (this.#position + diff < 0) { | ||
| this.#position = 0; | ||
| } else { | ||
| this.#position += diff; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
삼항 연산자로 바꿔도 좋을듯합니다!
| selectRandomMiniGame() { | ||
| const miniGameNames = Object.keys(this.miniGames); | ||
| this.currentMiniGame = | ||
| miniGameNames[getRandomNumber(0, miniGameNames.length - 1)]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
가독성을 높이기위해 변수에 할당하고 사용하면 좋지않을까요..?
const miniGameNames = ...;
const randomIndex = ...;
this.currentMiniGame = miniGameNames[randomIndex];이렇게요!
| this.maxRound = maxRound; | ||
| this.validate(this.maxRound, ['maxRoundNumber', 'maxRoundRange']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
유효성 검사를 하고 프로퍼티에 할당하는게 좋을듯합니다!
검사를 통과 못하면 할당할 필요가 없으니깐요!
| positions: this.cars.reduce( | ||
| (acc, car) => ({ | ||
| ...acc, | ||
| [car.name]: car.position, | ||
| }), | ||
| {}, | ||
| ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분은 분리해서 함수로 만들면 가독성이 높아질듯합니다!
| this.printer = new ConsolePrinter({ | ||
| roundStart: | ||
| '--------------------------⭐ Round%{1}⭐️️--------------------------', | ||
| carPosition: '%{1} : %{2} (%{3}%{4} ➡ %{5})', | ||
| gameLog: '%{1} : %{2} VS computer : %{3} ➡➡ %{4}', | ||
| miniGameStart: '>> player %{1} Turn!', | ||
| winner: '최종 우승자는 👑 %{1} 입니다. 축하합니다!', | ||
| error: '⚠️ %{1}', | ||
| divider: | ||
| '---------------------------------------------------------------', | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
템플릿 만드신건 정말 대단..!!!
| this.printer.printWithTemplate('gameLog', [ | ||
| name.padEnd(5, ' '), | ||
| ...Object.values(log), | ||
| ]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
displayGameLog 메서드를 만들면 가독성에 좋을것같아요!
| const { positions } = results[currentRound - 1]; | ||
| const prevPositions = | ||
| currentRound > 1 ? results[currentRound - 2].positions : {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
뭔가 인덱스로 참조해서 값을 가져오면 이해하기 어려워지는거 같아요.. ㅠ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
객체 다루기 마스터!
기본 추가 요구사항
추가한 요구사항
RacingGame 클래스에서 MiniGame 들을 받아서, 랜덤으로 미니게임을 진행하면서 전진하는 형식입니다 !
미션을 수행하며 어려웠던 점
다른 분들의 코드를 보고 뷰, 모델을 나누는 방식이 이 미션에 적합하다는 생각이 들었어요. 따라서 모델에서 반환된 게임 결과를 뷰가 받아서 출력하는 구조를 생각했었습니다 !
하지만 .. 제가 미니게임으로 일을 너무 크게 벌여놔서 중간 출력이 없으면 이상한 상황이 되었습니다 ! (가위바위보를 했는데 상대방이 뭘 냈는지 알 수가 없는..)
따라서 EventEmitter 객체를 데려와서, 모델에서 이벤트를 던지면 컨트롤러에서 받아서 뷰에게 출력 명령을 내리는 구조로 변경했습니다
출력하는 부분이 이제 뷰에 다 몰려있긴 하지만, console.log 안에 문자열이 산재되어 있으면 수정하기 어렵다는 생각이 들었어요.
상수로 관리할 수도 있지만, 다른 방법이 없을까.. 에 대해 고민을 했습니다.
따라서 printWithTemplate 이라는 메소드를 만들어서, Printer 객체를 생성할 때 템플릿 객체를 넣어줄 수 있도록 했어요 ! (맞는 방법인지는 모르겠습니다... ! 도전정신? 으로 해본 내용입니다)
예를 들면 아래와 같습니다.
사실 앞전 구현 내용(미니게임, 구조변경, 출력..) 에 시간을 너어어어무 뺏겨서 이 부분을 꼼꼼히 마무리하지 못했습니다 ㅠㅠ
객체에 검사 메소드인 check 와 에러때 출력할 errorMessage 를 작성하고, 이걸로 유효성 검사를 실행시켜주는 validator 를 반환하는 createValidator 를 구현했습니다. (사실 시간이 없어서... 객체만 작성하면 에러처리 함수를 일일히 안써도 되도록 하기 위함이었어요)
리뷰 받고 싶은 점 & 궁금한 점
이벤트를 활용하는 방식이 뷰와 로직을 분리한다는 관점에서 유의미한 방법인지 궁금합니다 !
이 외에도, 위에 작성한 부분들에 대해서 자유롭게 의견 주시면 감사하겠습니다 !
게임... 시간되면 다들 해보세요 .. ! 나름 재미있어요 🥹
내가 생각하는 클린코드의 원칙 한 가지
이 코드의 사용자를 고려했는가? 라고 생각합니다 ! 여기서 코드의 사용자란, 작성한 코드를 보고 수정하는 개발자입니다
결국 개발할 때의 여러가지 비용들을 줄여야 하기 때문에,
사용자에게 쉬운 서비스를 만드는 것 처럼 코드 작성도 개발자가 쉽게 사용할 수 있는 방향으로 해야한다고 생각합니다
구체적으로는, 코드를 수정할 때 스크롤 + 클릭 횟수 + 생각하는 비용을 적게 만들어야 된다 생각해요 !