[자동차 경주] 최재오 미션 제출합니다. #2
Conversation
| const inputNames = input.split(","); | ||
| if (inputNames.some((name) => name.length > STATIC_NUMBER.nameLengthLimit)) | ||
| throw new Error(ERROR_MESSAGE.nameLengthError); | ||
| if (inputNames.length !== new Set(inputNames).size) |
There was a problem hiding this comment.
맞습니다! set 객체는 중복되지 않는 유일한 값들의 집합이기 때문에 중복된 값은 나타나지 않습니다 👍🏻
| #name; | ||
| #distance = 0; | ||
|
|
||
| constructor(name) { |
There was a problem hiding this comment.
안녕하세요！ 코드를 읽다가 모르겠는 부분이 있어서 질문 남겨요. name 이랑 distance를 constructor 밖에서 선언했을 때랑 안에서 선언했을 때랑 혹시 다른 점이 있나요?？ 그리고 밖에 선언하신 이유가 밑에 다른 메서드들도 써야하기 때문일까요？
There was a problem hiding this comment.
생각해보니 #distance;로만 private field에 선언을 해두고 constructor 내부에서 0으로 초기화를 해도 될 것 같네요!
저는 배열을 순회하며 클래스 인스턴스를 받아올 때 이름만 필요하기 때문에 name만 넣었는데 distance를 넣어도 상관없을 것 같습니다 🙂
덕분에 하나 알아갑니다!
There was a problem hiding this comment.
기능별로 폴더를 나누신 것 같은데, 혹시 domain은 어떤 걸 말하는 지 알려주실 수 있나요？？ 다른 폴더명들은 다 이해가 가는데 domain 만 정확히 뭐를 나타내는 건지 모르겠어서요ㅜㅜ,,
There was a problem hiding this comment.
domain은 보통 MVC 디자인 패턴에서 Model을 의미합니다.
아직 저도 배울 점이 너무나 많지만 제 지식 내에서 설명을 드리자면 domain에서 보통 데이터 저장과 관리가 이루어집니다. 다른 일도 많이 하지만 저는 데이터 저장과 관리에 조금 힘을 많이 쏟았습니다. 그래서 controller에서 사용될 데이터들을 관리하고 가공하는 작업을 domain에서 했습니당 🙂
|
|
||
| async inputTries() { | ||
| await InputView.readTries((input) => { | ||
| this.#tries = Number(input); |
There was a problem hiding this comment.
#tries는 이 컨트롤러의 멤버 변수고, #cars는 멤버 변수이긴 하지만 다른 클래스의 인스턴스인 것 같은데요! #tries는 이 컨트롤러 내부에서 형변환 처리를 하고, #cars는 해당하는 클래스안에서 형변환을 해서 사용하는게(형변환 위치가 하나는 컨트롤러, 하나는 모델) 조금 일관적이지 않다고 느낀 것 같습니다. #tries도 클래스로 빼서 형변환을 다루는 것도 좋을 것 같아요 !
There was a problem hiding this comment.
우선 제가 마지막까지 고민했던 부분 짚어주셔서 너무 감사합니다 ㅜㅜ
사실 저 tries 변수를 RacingCars 클래스에서 처리를 했었는데 Car와 관련된 일관적인 변수라고 생각이 들지 않아서 컨트롤러에서 다뤘던 것 같습니다. 세현님 조언을 듣고 보니 차라리 tries를 클래스로 빼서 관리하는 것도 괜찮은 방법이었네요! 조언 감사합니다 ㅎㅎ
|
|
||
| async inputCarsName() { | ||
| await InputView.readCarsName((input) => { | ||
| this.setCars(input); |
There was a problem hiding this comment.
이 컨트롤러 클래스의 메서드 순서는 사실 컴파일러입장에서 중요하지 않지만, 사람이 읽는 입장에서는 접근하는 메서드 순서대로 작성해주시면 더 자연스럽게 읽기 편할 것 같습니다 ! 👍
There was a problem hiding this comment.
그 점을 생각 못한 것은 아닌데 input은 input끼리, race는 race끼리 통일성있게 모아뒀던 것 같네요.
읽는 입장을 생각하면 제가 했던 방법보다는 세현님이 말씀해주신 방법을 사용하는 것도 괜찮은 것 같아요! 감사합니다
|
|
||
| raceStart() { | ||
| OutputView.printResultMessage(); | ||
| Array.from({ length: this.#tries }).forEach(() => { |
There was a problem hiding this comment.
forEach 메서드를 사용하기 위해서 #tries 길이의 배열을 생성해 반복하는 것으로 보이는데요. #tries가 몇 억회로 입력될 경우 불필요한 공간이 낭비될 가능성이 있을 수 있습니다. 이렇게 극단적인 경우가 아니더라도, 배열의 forEach가 아닌 for나 while 반복문을 사용하는 것이 공간 낭비가 없고 직관적일 것 같아요 !
There was a problem hiding this comment.
최대한 고차함수를 적용하려다보니 이런 부분에서 문제가 발생할 수 있다는 점을 간과했습니다.
이렇게 입력값을 받아서 반복문을 돌릴 때에는 일반 for문을 이용하는 것이 더 메모리 측면에서 효율적일 것 같네요!
감사합니다 😊
| }); | ||
|
|
||
| const ERROR_MESSAGE = Object.freeze({ | ||
| nameLengthError: "[ERROR] 자동차의 이름은 5자를 넘어갈 수 없습니다.", |
There was a problem hiding this comment.
ERROR_MESSAGE 객체에 있는 프로퍼티 네임들도 끝에 Error가 붙어있는데요. 이미 ERROR_MESSAGE에서부터 에러에 관한 프로퍼티임을 알게되니까 프로퍼티 네임에까지 Error 를 붙이지 않아도 잘 알 수 있을 것 같습니다!
There was a problem hiding this comment.
생각치도 못한 곳에서 새로운 깨달음을 얻었네요...! 조언 정말 감사합니다 😭
| import OutputView from "../src/view/OutputView.js"; | ||
| import { Console } from "@woowacourse/mission-utils"; | ||
|
|
||
| describe("출력 값 테스트", () => { |
There was a problem hiding this comment.
저는 테스트하기 어렵게 메서드를 작성해서 미처 중간 과정을 테스트하지 못했는데, 테스트하기 좋은 코드로 깔끔하게 잘 작성하신 것 같아요 ! 👍 많이 배워갑니다. ㅎㅎ 저도 다음에 이렇게 테스트 할 수 있도록 노력해봐야 겠습니다. 재오님 코드는 전체적으로 깔끔했고, 완성도가 높다고 느낀 것 같아요.
조금만 첨언하자면, 테스트의 '-', '최종 우승자' 스트링을 Static.js에 선언해두신 moveMarking, winnerMessage 를 사용한다면 후추 변경 사항이 생겨도 테스트 코드를 수정하지 않아도 되므로 더 간편할 것 같습니다 !
코드리뷰 무지개반사 감사합니다 ㅎㅎ
| try { | ||
| const input = await Console.readLineAsync(PRINT_MESSAGE.inputTries); | ||
| InputValidator.validateTimesNumber(input); | ||
| callback(input); |
There was a problem hiding this comment.
callback 함수를 받아서 코드를 작성하셨는데 그냥 입력값을 받아서 async/await로 처리하는 것과 어떤 이점이 있는 건지 저는 후자의 방식으로 작성을 해서 궁금합니다 😉
There was a problem hiding this comment.
저도 다른 분의 코드를 많이 봤는데 보통 후자의 방식으로 작성하셨더라고요 ㅎㅎ
저는 우선 콜백 함수를 사용하면 코드를 더 모듈화할 수 있을 것 같아서 저 방식을 선택했습니다. 함수� 비동기 작업 완료 후 콜백을 호출하는 책임만 갖게 해 콜백 함수는 다른 결과를 처리하거나 추가 로직을 수행할 수 있는 것 같습니다 🙂
| triesInput.forEach((times) => { | ||
| expect(() => { | ||
| InputValidator.validateTimesNumber(times); | ||
| }).toThrow(); |
There was a problem hiding this comment.
'ERROR'로 시작하는 에러를 던져야 하는 요구 사항이 있으니 그것도 반영해서 테스트 코드에 넣어주시면 더 좋을 것 같아요 🤩
There was a problem hiding this comment.
헐 유선님이 짚어주셔서 확인할 수 있었던 부분이었습니다! 감사합니다
No description provided.