Conversation
Parkhanyoung
left a comment
There was a problem hiding this comment.
이번 한 주도 과제 완성하시느라 고생 많으셨어요!
전반적으로 잘 작성해주셔서 이해하는 데에는 큰 무리가 없었습니다.
하지만 만약 로직을 여러 메소드로 더 나눠주셨다면 가독성이 더 좋아지지 않을까 하는 생각은 들었던 것 같아요!
이번 주차에 로직들을 잘게 나눠보고, 가능하면 다른 파일로 분리해보는 연습을 해보시면 도움이 될 것 같습니다!
다음주도 같이 화이팅해봐요 💪🏻
| class App { | ||
| async play() {} | ||
|
|
||
| nameValidation(input) { |
There was a problem hiding this comment.
| nameValidation(input) { | |
| validateCarNames(input) { |
일부 케이스를 제외하고는 보통 메소드나 함수명은 동사형으로 짓는 것 같은데, 동사형 이름은 어떠신가요?
| if (input.includes(' ')) throw new Error("[ERROR] 공백을 제외하고 입력해주세요."); | ||
| if (input.includes(',,')) throw new Error("[ERROR] 쉼표가 연달아 입력되었습니다."); | ||
| if (inputArray.length <= 1) throw new Error("[ERROR] 2개 이상의 자동차가 있어야 시작할 수 있습니다."); | ||
| inputArray.forEach((item) => { | ||
| if (item.length > 5) throw new Error("[ERROR] 5자 이하로 입력해주세요."); | ||
| if (names.has(item)) throw new Error("[ERROR] 중복된 이름이 있습니다."); |
There was a problem hiding this comment.
이 검증 로직들을 또 별도의 메소드로 쪼개볼 수 있을 것 같은데, 더 많은 메소드로 쪼개보는 건 어떠신가요?
There was a problem hiding this comment.
잘게 나누면 로직이 일정 구간 별로 묶이면서 읽는 입장에서 이해하기가 수월하더라구요
|
|
||
| async play() { | ||
| try { | ||
| const stuffs = await this.playerInput(); |
There was a problem hiding this comment.
혹시 stuffs라는 변수명을 지으신 이유가 있으신가요~? 변수명은 최대한 명확한 게 좋다고 들어서 stuffs와 같이 불분명한 이름 대신 확실하게 의미가 드러나는 단어로 지어보시면 가독성에 도움이 될 것 같습니다!
또, 자바스크립트에서 배열로부터 값을 꺼내는 아래와 같은 문법도 있으니 참고해보시면 좋을 것 같아요!
| const stuffs = await this.playerInput(); | |
| const [names, rounds] = await this.playerInput(); | |
| let i = rounds; | ||
| while(i != 0) { | ||
| names.forEach((value, key, map) => { | ||
| const num = MissionUtils.Random.pickNumberInRange(0, 9); |
| names.forEach((value, key, map) => { | ||
| const num = MissionUtils.Random.pickNumberInRange(0, 9); | ||
| if (num >= 4) map.set(key, value+"-"); | ||
| MissionUtils.Console.print(`${key} : ${map.get(key)}`); |
| } | ||
|
|
||
| diceLogic(names, rounds) { | ||
| let i = rounds; |
There was a problem hiding this comment.
round 수와 같은 정보는 App 클래스의 프로퍼티로 다루는 것도 괜찮을 것 같다는 생각이 들었습니다!
|
감사합니다! 이번 주 과제에 적용해서 진행해보겠습니다!
-----Original Message-----
From: ***@***.***>
To: ***@***.***>;
Cc: ***@***.***>; ***@***.***>;
Sent: 2023-11-03 (금) 00:15:17 (GMT+09:00)
Subject: Re: [woowacourse-precourse/javascript-racingcar-6] [자동차 경주] 박건홍 미션 제출합니다. (PR #737)
@Parkhanyoung commented on this pull request.
이번 한 주도 과제 완성하시느라 고생 많으셨어요!
전반적으로 잘 작성해주셔서 이해하는 데에는 큰 무리가 없었습니다.
하지만 만약 로직을 여러 메소드로 더 나눠주셨다면 가독성이 더 좋아지지 않을까 하는 생각은 들었던 것 같아요!
이번 주차에 로직들을 잘게 나눠보고, 가능하면 다른 파일로 분리해보는 연습을 해보시면 도움이 될 것 같습니다!
다음주도 같이 화이팅해봐요 💪🏻
In src/App.js:
class App { - async play() {} + + nameValidation(input) { ⬇️ Suggested change - nameValidation(input) { + validateCarNames(input) {
일부 케이스를 제외하고는 보통 메소드나 함수명은 동사형으로 짓는 것 같은데, 동사형 이름은 어떠신가요?
In src/App.js:
+ if (input.includes(' ')) throw new Error("[ERROR] 공백을 제외하고 입력해주세요."); + if (input.includes(',,')) throw new Error("[ERROR] 쉼표가 연달아 입력되었습니다."); + if (inputArray.length <= 1) throw new Error("[ERROR] 2개 이상의 자동차가 있어야 시작할 수 있습니다."); + inputArray.forEach((item) => { + if (item.length > 5) throw new Error("[ERROR] 5자 이하로 입력해주세요."); + if (names.has(item)) throw new Error("[ERROR] 중복된 이름이 있습니다.");
이 검증 로직들을 또 별도의 메소드로 쪼개볼 수 있을 것 같은데, 더 많은 메소드로 쪼개보는 건 어떠신가요?
In src/App.js:
+ ranking.forEach((value, index, array) => { + if (index === 0) winners.push(value[0]); + if (index > 0 && array[0][1] === value[1]) winners.push(value[0]); + } + ); + return winners; + } + + announceWinner (winners){ + const announce = winners.join(', '); + MissionUtils.Console.print(`최종 우승자 : ${announce}`); + } + + async play() { + try { + const stuffs = await this.playerInput();
혹시 stuffs라는 변수명을 지으신 이유가 있으신가요~? 변수명은 최대한 명확한 게 좋다고 들어서 stuffs와 같이 불분명한 이름 대신 확실하게 의미가 드러나는 단어로 지어보시면 가독성에 도움이 될 것 같습니다!
또, 자바스크립트에서 배열로부터 값을 꺼내는 아래와 같은 문법도 있으니 참고해보시면 좋을 것 같아요!
⬇️ Suggested change - const stuffs = await this.playerInput(); + const [names, rounds] = await this.playerInput();
In src/App.js:
+ const nameinput = await MissionUtils.Console.readLineAsync('경주할 자동차 이름을 입력하세요.(이름은 쉼표(,) 기준으로 구분)\n'); + const names = this.nameValidation(nameinput); + const numberinput = await MissionUtils.Console.readLineAsync('시도할 횟수는 몇 회인가요?'); + const rounds = this.roundValidation(numberinput); + return [names, rounds]; + } catch(error){ + MissionUtils.Console.print(error.message); + throw new Error(); + } + } + + diceLogic(names, rounds) { + let i = rounds; + while(i != 0) { + names.forEach((value, key, map) => { + const num = MissionUtils.Random.pickNumberInRange(0, 9);
이 랜덤 수를 뽑는 로직을 별도의 함수로 분리하면 어떨까요?
In src/App.js:
+ const numberinput = await MissionUtils.Console.readLineAsync('시도할 횟수는 몇 회인가요?'); + const rounds = this.roundValidation(numberinput); + return [names, rounds]; + } catch(error){ + MissionUtils.Console.print(error.message); + throw new Error(); + } + } + + diceLogic(names, rounds) { + let i = rounds; + while(i != 0) { + names.forEach((value, key, map) => { + const num = MissionUtils.Random.pickNumberInRange(0, 9); + if (num >= 4) map.set(key, value+"-"); + MissionUtils.Console.print(`${key} : ${map.get(key)}`);
결과를 출력하는 이 부분도 분리해볼 수 있을 것 같아요!
In src/App.js:
+ + async playerInput() { + try{ + const nameinput = await MissionUtils.Console.readLineAsync('경주할 자동차 이름을 입력하세요.(이름은 쉼표(,) 기준으로 구분)\n'); + const names = this.nameValidation(nameinput); + const numberinput = await MissionUtils.Console.readLineAsync('시도할 횟수는 몇 회인가요?'); + const rounds = this.roundValidation(numberinput); + return [names, rounds]; + } catch(error){ + MissionUtils.Console.print(error.message); + throw new Error(); + } + } + + diceLogic(names, rounds) { + let i = rounds;
round 수와 같은 정보는 App 클래스의 프로퍼티로 다루는 것도 괜찮을 것 같다는 생각이 들었습니다!
In src/App.js:
+ if (input.includes(' ')) throw new Error("[ERROR] 공백을 제외하고 입력해주세요."); + if (input.includes(',,')) throw new Error("[ERROR] 쉼표가 연달아 입력되었습니다."); + if (inputArray.length <= 1) throw new Error("[ERROR] 2개 이상의 자동차가 있어야 시작할 수 있습니다."); + inputArray.forEach((item) => { + if (item.length > 5) throw new Error("[ERROR] 5자 이하로 입력해주세요."); + if (names.has(item)) throw new Error("[ERROR] 중복된 이름이 있습니다.");
잘게 나누면 로직이 일정 구간 별로 묶이면서 읽는 입장에서 이해하기가 수월하더라구요
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
No description provided.