[2단계 - 자동차 경주 리팩터링] - 윤생(이윤성) 미션 제출합니다. #204
Conversation
- @woowacourse/mission-utils 설치
Co-authored-by: Gabriel Ju Hyun, Yoon <gabrielyoon7@gmail.com>
Co-authored-by: Gabriel Ju Hyun, Yoon <gabrielyoon7@gmail.com>
Co-authored-by: Gabriel Ju Hyun, Yoon <gabrielyoon7@gmail.com>
- 원래 콜백패턴에서 사용해 의존성이 입력 로직에 얽혀있었는데, play 메서드로 이동했다.
- UI 로직과 도메인 로직을 담당하는 모듈 분리
- tryMoveCars 에서 "실행 결과"를 실행시키는 부분을 전체 실행을 관장하는 play로 빼냄 - tryMoveCars는 차들을 움직이는 도메인 로직만 가지고 있어야 한다고 판단
There was a problem hiding this comment.
윤생 안녕하세요!
2단계 미션까지 수고 많으셨습니다. 1단계에서 워낙 깔끔하게 코드를 작성해놓으셔서 그런가 2단계 코드도 읽는데 큰 어려움이 없었네요. 😄
피드백은 딱 하나 남겨두었습니다. 확인해주시고 피드백 반영해주시면 감사드릴게요.
단위 테스트 에서 사용한 jest.fn()을 삭제하고, 몇 가지 단위 테스트를 새로 짜보았습니다. 이번에는 테스트 제목을 살짝 바꾸보았어요. 제 생각에는 테스트 제목을 짓는 것도 정말 중요한 것 같아요. 혹시 이것에 대해 체프는 어떻게 생각하시나요?
네, 저도 테스트 제목을 잘 짓는 것 또한 중요하다고 생각합니다. 물론 테스트 코드에 맞도록 제목을 잘 지어야 할 것입니다.
테스트 제목을 어떻게 하면 잘 지을 수 있을 지 생각해보시는 것도 좋을 것 같습니다.
테스트 제목을 짓는 것을 보통 테스트 케이스를 작성한다고 표현하는데요. 보통 테스트 코드를 짤 때는 이러한 테스트 케이스를 먼저 작성하고, 이에 맞추어서 테스트 코드를 짜게 됩니다. 그래서 테스트 제목을 처음에 잘 작성하는 것이 중요하다고 생각합니다.
개인적으로 선호하는 방법은 유저의 행동 시나리오를 가정해서 작성하는 것입니다.
예를 들어서 시도 횟수 입력이 유효하지 않으면 에러를 던지는 지 테스트의 경우에는 다음과 같이 변경해볼 수 있습니다.
유효하지 않은 시도 횟수를 입력하면 에러가 반환된다.
유저의 행동, 그리고 그에 따라 유저가 받게 되는 결과를 명시하고, 이를 테스트하는 것입니다. 유저 입장에서 테스트를 작성하게 되는 것이죠.
나중에 DOM을 다루실 때 이러한 방식으로 테스트 케이스를 작성하시면 도움이 될 것이라고 생각합니다. 유저가 어떤 텍스트를 입력했는지 가정하고 어떤 에러 메시지가 DOM에 출력되었는지 직접 테스트 코드를 짜실 수 있게 될테니까요.
src/utils/Validation.js
Outdated
| let count = 0; | ||
| // eslint-disable-next-line no-unused-vars | ||
| for (const char of carName) { | ||
| count += 1; | ||
| } | ||
| return this.isValidCarNameLength(count); |
There was a problem hiding this comment.
코드를 작성하는 것은 또 다른 의미의 문서화라는 말을 들어보신 적이 있으신가요?
윤생과 저는 이모지 예외 케이스를 해결하기 위해서 작성한 코드라는 것을 알고 있지만, 처음 이 코드를 읽는 사람은 어떻게 생각할까요?
이 코드가 어떤 코드인지 알 수 있게 하려면 어떻게 하는 게 좋을까요?
There was a problem hiding this comment.
저 로직을 함수로 분리하면 좋겠다고 생각이 듭니다! 한번 시도해보겠습니다!
| @@ -21,25 +21,24 @@ class GameManager { | |||
|
|
|||
| printCars(cars) { | |||
| cars.forEach((car) => { | |||
| car.print(); | |||
| OutputView.printCar(car.name, car.position); | |||
There was a problem hiding this comment.
car가 직접 스스로에 대한 정보를 print하는 것보다 이렇게 OutputView의 메소드인 printCar가 출력하는 구조가 훨씬 깔끔하고 좋은 것 같습니다. 역할과 책임이 좀 더 명확하게 구분된 것 같네요. 👍
| @@ -84,6 +83,7 @@ class GameManager { | |||
| async play() { | |||
| await this.handleCarNames(); | |||
| const tryCount = await this.handleTryCount(); | |||
| OutputView.printResult(); | |||
There was a problem hiding this comment.
적절한 위치에 OutputView.printResult() 코드가 삽입된 것 같습니다. 잘 리팩토링하신 것 같네요. 👍
| export default function calcWordCount(text) { | ||
| let count = 0; | ||
| // eslint-disable-next-line no-unused-vars | ||
| for (const char of text) { | ||
| count += 1; | ||
| } | ||
| return count; | ||
| } |
There was a problem hiding this comment.
적절한 네이밍과 함께 함수를 분리하셨네요! 좋습니다 👍
개인적으로는 이모지와 같은 예외 상황에 대응하기 위해 for ... of문을 사용하였다는 Note 주석을 남겨두는 것도 괜찮은 방법일 것 같아요.
안녕하세요 체프! 주말은 잘 보내셨나요? 벌써 일요일이 30분 밖에 안 남았네요ㅠ
이번 주도 화이팅입니다!! 😊
2단계에서 고민한 내용은 다음과 같습니다.
단위 테스트 에서 사용한
jest.fn()을 삭제하고, 몇 가지 단위 테스트를 새로 짜보았습니다. 이번에는 테스트 제목을 살짝 바꾸보았어요. 제 생각에는 테스트 제목을 짓는 것도 정말 중요한 것 같아요. 혹시 이것에 대해 체프는 어떻게 생각하시나요?1단계 미션에서 언급한 서로게이트 쌍을 해결하기 위해 유효성 검증 부분 로직을 변경하였습니다 ! 체프가 알려주신 방법도 있었지만
for..of를 쓰는게 좀 더 간결하고 이해하기 쉬워서 이 방법으로 했어요. (아직 이터레이터가 어렵더군요 ㅠㅠ 차차 공부해서 다른 미션에서 사용할 기회가 있다면 적용해보도록 하겠습니다.)제 생각에는
GameManager는 어쩔 수 없이 도메인 로직과 UI 로직을 함께 가지고 있어야 된다고 생각해요. 그러다 보니 클래스가 커지는거 같아 속에서 도메인 로직을 분리해야 할지 고민입니다.하지만 지금 생각으로는 이 정도는 가지고 있어야 될 것 같다는 생각이 더 커 클래스를 분리하기 보다는 도메인 로직만 가지고 있는 메서드 내에서 필요없는 UI 로직을 외부로 분리했습니다. 해당 커밋: (refactor: 메서드 책임 분리))
이상입니다! 피드백 부탁드리겠습니다 👍