-
Notifications
You must be signed in to change notification settings - Fork 212
[자동차 경주] 강성준 미션 제출합니다. #205
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: main
Are you sure you want to change the base?
Conversation
요구사항을 바탕으로 구조화한 설계와 기능 목록을 정리하고, 설계 단계에서 고민한 내용들을 정리했습니다.
기능 목록의 검증 기능을 따로 두어 세분화하였습니다.
woowacourse/mission-utils의 Console을 사용하여 사용자로 부터 입력을 받는 함수를 구현하였습니다. 해당 함수를 사용해 받는 입력은 쉼표로 구분된 자동차명과 진행될 랩 수입니다.
입력받은 자동차를 쉼표를 기준으로 파싱하기 위해서 parseByComma 함수를 구현하였습니다.
missionutil의Random을사용해 무작위 수를 생성하는 함수를 추가하였습니다. 그리고 해당 함수를 호출하여 필요로 하는 개수(차량의 수*랩 수)의 난수 배열을 반환하는 함수를 구현했습니다.
자동차명, 랩수, 난수를 받아 경주를 진행하는 도메인 함수를 구현하였습니다. 반환값은 경주 히스토리와 최종 결과를 모두 사용할 수 있도록 랩을 기준으로 분리된 2차원 배열로 결정했습니다.
최종 점수를 가지고 가장 높은 점수를 뽑아 우승자를 결정하는 함수를 구현하였습니다. 동률일 경우 공동 우승으로 처리됩니다.
utils/parsing에서 변환한 히스토리 로그를 app에서 mission util api로 출력하는 함수를 구현했습니다.
우승자를 문자열로 파싱한 뒤 misson util로 우승자를 출력하는 함수를 구현하였습니다. 우승자가 여러 명일 경우 쉼표로 구분되어 출력됩니다.
처음에 주어진 테스트 케이스를 통과하지 못하는 이유가 생성한 난수 배열을 stack의 pop()을 통해 값을 하나씩 가져오고 있기 때문이었습니다. 그래서 FIFO로 읽어 올 수 있도록 shift()를 사용하도록 수정하였습니다.
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.
_
| describe("레이스 진행 도메인 테스트", () => { | ||
| test.each([ | ||
| [["a", "b", "c"], 2, [4, 1, 2, 9, 0, 4], [[1, 0, 0], [2, 0, 1]]], | ||
| [["red", "blue", "black"], 3, [5, 2, 4, 9, 5, 9, 9, 4, 6], [[1, 0, 1], [2, 1, 2], [3, 2, 3]]] |
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 MOVE = 4 const STOP = 3
과 같은 방식으로 상수화 해서 테스트코드를 깔끔하게 하신분도 있더라고요 추천드립니다!
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.
생각도 못해 본 부분이네요! 각 테스트의 규칙적이고 필요한 이유에 대해서 생각해보겠습니다. 새로운 방식 추천도 감사드립니다 ㅎㅎ
| class App { | ||
| async run() { | ||
| // 입력(자동차명, 횟수) | ||
| const stringOfCarNamesUserRequest = await this.readInputAsyncUsingWoowaMissionApi("경주할 자동차 이름을 입력하세요.(이름은 쉼표(,) 기준으로 구분)"); |
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.
제가 봐도 조금 과한 부분인 것 같습니다. 타인이나 나중에 제가 코드를 다시 본다면 이해할 수 있을까를 고민하다가, 최대한 디테일하게 이름을 지어보자는 의도였습니다. getInputWoowaMissionApi나 readAsyncWoowaAPI 정도도 충분할 것 같습니다. 이것도 과할까요 ㅋㅋㅋ ;;
| validateCarNameRule(arrayOfCarNamesUserRequest); | ||
| const stringOfLapUserRequest = await this.readInputAsyncUsingWoowaMissionApi("시도할 횟수는 몇 회인가요?"); | ||
| const countOfLapUserRequest = Number(stringOfLapUserRequest); | ||
| validateLapNumberRule(countOfLapUserRequest); |
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 countOfLapUserRequest = Number(stringOfLapUserRequest); | ||
| validateLapNumberRule(countOfLapUserRequest); | ||
| // 레이스 진행 | ||
| const randomNumberTape = this.makeRandomNumbersTape(arrayOfCarNamesUserRequest, countOfLapUserRequest); |
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.
미리 개수만큼 랜덤을 뽑는건가요 ?
미리 하신 이유가 있을까요 ?
race 의 책임 경계에서 고민을 많이하셨을거 같습니다
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.
레이스를 진행하는 부분을 순수함수로 구현하고자 함이었습니다. 그렇다 보니 입력, 랜덤과 같은 부분을 분리하려고 했습니다. 메모리에 대한 트레이드오프가 있지만 테스트할 때 매우 유용했습니다.
| @@ -0,0 +1,8 @@ | |||
| export const findMaxValue = (values) => Math.max(...values); // 명확성과 재사용성이 높아보여 분리 | |||
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.
전체적으로 너무 잘 작성되고 가독성 좋은코드라 보기 좋았습니다
특히 라이브러리 테스트 코드를 작성하신게 인상적이네요
좋은코드 많이배웠습니다!
이메일을 잘못남겨서 알림이 안오더라고요 .. 동일한 내용으로 리뷰남겼습니다 죄송해요 ㅠ
| describe("레이스 진행 도메인 테스트", () => { | ||
| test.each([ | ||
| [["a", "b", "c"], 2, [4, 1, 2, 9, 0, 4], [[1, 0, 0], [2, 0, 1]]], | ||
| [["red", "blue", "black"], 3, [5, 2, 4, 9, 5, 9, 9, 4, 6], [[1, 0, 1], [2, 1, 2], [3, 2, 3]]] |
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 MOVE = 4 const STOP = 3
과 같은 방식으로 상수화 해서 테스트코드를 깔끔하게 하신분도 있더라고요 추천드립니다!
| class App { | ||
| async run() { | ||
| // 입력(자동차명, 횟수) | ||
| const stringOfCarNamesUserRequest = await this.readInputAsyncUsingWoowaMissionApi("경주할 자동차 이름을 입력하세요.(이름은 쉼표(,) 기준으로 구분)"); |
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.
함수명을 조금은 줄여도 되지 않을까 싶습니다...
의미를 그대로 살리기 위해서 함수명을 줄이지 않는것이 중요하지만
다소 길거나 불필요한것 같기도 한 부분이 존재하는것 같아요
| validateCarNameRule(arrayOfCarNamesUserRequest); | ||
| const stringOfLapUserRequest = await this.readInputAsyncUsingWoowaMissionApi("시도할 횟수는 몇 회인가요?"); | ||
| const countOfLapUserRequest = Number(stringOfLapUserRequest); | ||
| validateLapNumberRule(countOfLapUserRequest); |
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 countOfLapUserRequest = Number(stringOfLapUserRequest); | ||
| validateLapNumberRule(countOfLapUserRequest); | ||
| // 레이스 진행 | ||
| const randomNumberTape = this.makeRandomNumbersTape(arrayOfCarNamesUserRequest, countOfLapUserRequest); |
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.
미리 개수만큼 랜덤을 뽑는건가요 ?
미리 하신 이유가 있을까요 ?
race 의 책임 경계에서 고민을 많이하셨을거 같습니다
| @@ -0,0 +1,8 @@ | |||
| export const findMaxValue = (values) => Math.max(...values); // 명확성과 재사용성이 높아보여 분리 | |||
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.
당연하다고 생각한 의존성까지 테스트 하신 부분이 인상깊습니다 :)
| [2.1, "[ERROR] : 횟수는 양의 정수이어야 합니다."], | ||
| [-1, "[ERROR] : 횟수는 양의 정수이어야 합니다."], | ||
| [NaN, "[ERROR] : 횟수는 양의 정수이어야 합니다."], | ||
| [null, "[ERROR] : 횟수는 양의 정수이어야 합니다."], | ||
| [undefined, "[ERROR] : 횟수는 양의 정수이어야 합니다."], |
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.
3주차 미션 때 상수화 부분을 적용해보려고 합니다. 감사합니다!!
| export function validateCarNameRule(names) { | ||
| if(new Set(names).size != names.length) throw new Error("[ERROR] : 중복된 이름을 사용할 수 없습니다.") | ||
| names.forEach(n => { | ||
| if(n.length > 5 || n.length === 0) throw new Error("[ERROR] : 자동차 명은 5자 이하여야 합니다."); | ||
| }); | ||
| } |
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.
자동차 이름이 없어도 "[ERROR] : 자동차 명은 5자 이하여야 합니다." 라는 에러가 뜹니다!
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.
앗! 검증 부분을 더 꼼꼼히 체크해야겠습니다. 찾아주셔서 감사합니다!
| export function validateLapNumberRule(lap) { | ||
| if(lap <= 0 || !Number.isInteger(lap)) | ||
| throw new Error("[ERROR] : 횟수는 양의 정수이어야 합니다.") | ||
| } No newline at end of file |
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.
분리 처리 후 다른 에러 메세지를 준다면 사용자 입장에서 더 편할 것 같네요! 감사합니다~
| export function parseToHistoryFormat(oneRoundHistory, name) { // [1,1,1] ["a","b","c"] | ||
| return oneRoundHistory | ||
| .map((result, idx) => `${name[idx]} : ${"-".repeat(result)}`) | ||
| .join("\n"); | ||
| } | ||
|
|
||
| export function parserToWinnerFormat(winner) { | ||
| return `최종 우승자 : ${winner.join(", ")}`; | ||
| } No newline at end of file |
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.printWinnerOfRace(namesOfWinner); | ||
| } | ||
|
|
||
| async readInputAsyncUsingWoowaMissionApi(questionStr) { |
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.
명칭을 구체화하는 과정에서 오히려 가독성이 떨어진다고 느껴집니다!
| makeRandomNumbersTape(cars, laps) { // 부수효과를 없애기 위해 필요한 만큼의 난수를 만들고 레이스 진행 | ||
| let numberTape = []; | ||
| const countOfNeededNumber = cars.length * laps; | ||
| for(let cnt = 0; cnt < countOfNeededNumber; cnt++) { | ||
| numberTape.push(this.pickRandomNumberInRangeUsingWoowaMissionApi(0, 9)) | ||
| } | ||
| return numberTape; | ||
| } |
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.
레이스를 진행하는 부분을 순수함수로 구현하고자 함이었습니다. 그렇다 보니 입력, 랜덤과 같은 부분을 분리하려고 했습니다. 메모리에 대한 트레이드오프가 있지만 테스트할 때 매우 유용했습니다.
기능 목록
,기준으로 자동차명을 구분[ERROR]로 시작하는 문구와 함께 에러를 발생시키고 프로그램을 종료시킨다.자동차 수 * 랩 수만큼의 랜덤 숫자(0~9사이 정수) 생성,로 구분하여 출력한다.문제 해결 과정
설계 단계
App 클래스가부수효과(입출력, API)를 모두 담당하고 레이스를 진행하는 부분은 순수함수로 작성하려고 합니다. 이를 바탕으로 함수명 구조를 짜보았는데, 아마 구현을 진행하면서 많은 수정이 있을 것 같습니다.구현 단계