-
Notifications
You must be signed in to change notification settings - Fork 693
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
[자동차 경주] 박건홍 미션 제출합니다. #737
base: main
Are you sure you want to change the base?
Conversation
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 play() {} | ||
|
||
nameValidation(input) { |
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.
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.
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.
잘게 나누면 로직이 일정 구간 별로 묶이면서 읽는 입장에서 이해하기가 수월하더라구요
|
||
async play() { | ||
try { | ||
const stuffs = await this.playerInput(); |
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.
혹시 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); |
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.
이 랜덤 수를 뽑는 로직을 별도의 함수로 분리하면 어떨까요?
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)}`); |
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.
결과를 출력하는 이 부분도 분리해볼 수 있을 것 같아요!
} | ||
|
||
diceLogic(names, rounds) { | ||
let i = rounds; |
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.
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.