-
Notifications
You must be signed in to change notification settings - Fork 627
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
[로또] 정나리 미션 제출합니다. #643
base: main
Are you sure you want to change the base?
[로또] 정나리 미션 제출합니다. #643
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.
코드 리뷰 완료
//맞춘 개수 카운트 | ||
checkWinningCount(winningNumList) { | ||
let coincideNumCount = 0; | ||
for (var i = 0; i < 6; i++) { |
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.
var이 아닌 ES6에 도입된 let을 사용해 주세요
var 키워드는 함수 레벨 스코프이며, 같은 스코프 내에서 변수 중복 선언이 가능하다는 단점이 존재합니다
또한 var 키워드의 변수는 선언 단계와 초기화 단계가 한번에 진행됩니다
이로 인해, var로 선언한 변수는 런타임 이전에 선언 및 초기화가 동시에 이루어지기에 변수 선언문 이전에 참조가 가능하며, 할당문 이전에 변수를 참조하면 언제나 undefined를 반환합니다
console.log(foo); // undefined
var foo; // 런타임 이전에 변수 선언 및 초기화 단계
console.log(foo); // undefined
// 변수에 값을 할당하는 할당 단계
foo = 123;
console.log(foo); // 123
이는 프로그램의 흐름상 맞지 않으며 가독성을 떨어뜨리고 오류를 발생시킬 여지를 남김니다
이에 반해 let은 블록 레벨 스코프이며 같은 스코프 내에서 변수 중복 선언이 금지되어 있습니다
또한 let 키워드로 선언한 변수는 선언 단계와 초기화 단계가 분리되어 진행됩니다
암묵적으로 런타임 이전에 선언 단계가 먼저 실행되지만, 초기화 단계는 변수 선언문에 도달했을 때 실행되기에 변수를 선언하기 이전에 참조할 경우, RefereceError가 발생합니다
console.log(foo); // ReferenceError
// 변수 선언문에서 초기화 단계 실행
let foo;
console.log(foo); // undefined
// 할당문에서 할당 단계 실행
foo = 1;
console.log(foo); // 1
동작 과정
1. 런타임 이전에 js 엔진에 의해 변수 foo 등록(이때 foo 변수의 할당된 값은 존재하지 않는다)
2. 런타임 시작
3. foo 변수가 초기화 되지 않았으므로 ReferenceError 발생
4. let foo에서 초기화 단계 실행
5. foo에 초깃값 undefined가 존재하니 undefined 출력
6. 할당 단계에서 foo에 1 할당
7. 1 출력
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.
App 클래스의 기능이 너무 방대한 것 같습니다. 유사한 기능끼리 서로 묶어 모듈로 만들어 App 클래스의 책임을 분산하는 것이 나아 보입니다
async play() { | ||
//구입 금액 입력받는 함수 | ||
async function getUserCostInput() { | ||
const isNumber = (pVal) => { |
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.
pVal 매개변수가 무슨 의미이지 잘 모르겠습니다
매개변수 이름을 명확하게 명시하면 좋겠습니다
또한 isNumber 함수가 어떤 역할을 하는지 궁금합니다
for (let n = 0; n < numbers.length; n++) { | ||
for (let m = n + 1; m < numbers.length; m++) { | ||
if (numbers[m] === numbers[n]) { | ||
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.
함수의 depth가 3입니다
우테코 3주차 요구사항인
indent(인덴트, 들여쓰기) depth를 3이 넘지 않도록 구현한다. 2까지만 허용한다.
위배되는 것 같습니다
if (isEmpty(pVal)) { | ||
return false; | ||
} | ||
return !isNaN(pVal); |
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.
isNaN 보다 더욱 신뢰성 있는 Number.isNaN을 고려해 보세요
totalLottoNumList, | ||
winningNumList | ||
) { | ||
const totalWinningCount = [0, 0, 0, 0, 0, 0, 0, 0]; |
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.
totalWinningCount의 0번째 인덱스 ~ 2번째 인덱스까지 사용하지 않고 있습니다
배열이 아닌 객체로 생성하는 것은 어떨까요?
cosnt totalWinningCount = {first: 0, second: 0, third: 0 fourth: 0, fifth: 0}
const totalCost = await getUserCostInput(); | ||
const boughtLottoCount = totalCost / 1000; | ||
const totalLottoNumList = makeNewLotto(boughtLottoCount); | ||
printNewLotto(boughtLottoCount, totalLottoNumList); | ||
const winningNumList = await getUserWinningNumInput(); | ||
const bonusNum = await getUserBonusNumInput(); | ||
const [totalWinningCount, totalWinningMoney] = calcTotalWinningCount( | ||
boughtLottoCount, | ||
totalLottoNumList, | ||
winningNumList | ||
); | ||
printTotalNumber(totalWinningCount); | ||
const totalProfit = calcTotalProfitPercent(totalWinningMoney, totalCost); | ||
|
||
//당첨 수익률 출력 | ||
MissionUtils.Console.print(`총 수익률은 ${totalProfit}%입니다.`); |
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.
함수를 호출하는 부분이 너무 방대합니다
코드를 분리하여 App 클래스의 책임을 분산시키면 좋겠습니다
throw new Error("[ERROR] 로또 번호는 6개여야 합니다."); | ||
} | ||
for (let n = 0; n < numbers.length; n++) { | ||
for (let m = n + 1; m < numbers.length; m++) { | ||
if (numbers[m] === numbers[n]) { | ||
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.
3주차 미션에서의 기능 요구 사항에서
사용자가 잘못된 값을 입력할 경우 throw문을 사용해 예외를 발생시킨다. 그런 다음, "[ERROR]"로 시작하는 에러 메시지를 출력하고 해당 부분부터 입력을 다시 받는다.
이렇게 나와 있는데, 코드를 보시면 에러를 출력한 다음 입력을 다시 받는 것이 아닌 에러를 던진 후, 프로그램이 종료됩니다
calcGetMoney(coincideNumCount, bonusTrue) { | ||
if (coincideNumCount === 3) { | ||
return 5000; | ||
} | ||
|
||
if (coincideNumCount === 4) { | ||
return 50000; | ||
} | ||
|
||
if (coincideNumCount === 5) { | ||
if (!bonusTrue) return 1500000; | ||
return 30000000; | ||
} | ||
|
||
if (coincideNumCount === 6) { | ||
return 2000000000; | ||
} | ||
return 0; | ||
} |
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.
if문이 많을 경우 switch문을 고려해 보세요
함수의 길이가 15줄 초과입니다
const totalWinningCount = [0, 0, 0, 0, 0, 0, 0, 0]; | ||
let totalWinningMoney = 0; | ||
|
||
for (let j = 0; j < boughtLottoCount; j++) { |
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.
변수명을 j가 아닌 의도가 드러나도록 명확하게 입력해 주세요.
[로또] 정나리 미션 제출합니다.