-
Notifications
You must be signed in to change notification settings - Fork 426
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
[로또] 김지환 미션 제출합니다. #386
base: main
Are you sure you want to change the base?
[로또] 김지환 미션 제출합니다. #386
Conversation
543c1a2
to
7a221c3
Compare
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.
LGTM
코드를 금방 구현하셨다고 들었는데 깔끔히 잘 한 것 같아요 !
다만 LottoVendingMachine에 너무 많은 로직이 들어가 있는 것 같아 이 부분만 분리를 하면 더 좋을 것 같습니다 ! 수고하셨습니다
if (Number(number) < 1000) { | ||
throw new Error(MESSAGES.ERROR.PREFIX + MESSAGES.ERROR.SMALL_INT); | ||
} | ||
if (number.slice(-3) !== "000") { |
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.
방법은 되게 새로웠는데 로또 가격이 1000원이 아닐 경우 경우의 수가 많아지기 때문에 좋은 방법인진 모르네요..🤔
1000원 이외의 금액으로 바뀌었을 때 좋은 방법인지 고민해봐야될 것 같아요 ! 혹시 방법이 따로 있을까요?
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.#validate(numbers); | ||
this.#validateDuplicate(numbers); |
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.
validate안에 validateIntsRange 는 validate안에 넣었는데 validateDuplicate를 따로 하신 이유가 있을까요?!
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.
- validateIntsRange()는 number 배열이 아니라 파라미터로 하나의 숫자를 받아 검증하도록 해서였습니다
- 원랜 같이 넣으려다가 너무 함수가 길어져서 뺐습니다.
- 차라리 validate들을 더 분리하고 생성자에서 다 불러오는 것이 가독성 측면에서 더 좋아 보였는데 시간이 없었네요 :)
Console.print(""); | ||
Console.print(MESSAGES.OUTPUT.WIN_RATE); |
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.
메세지 자체에 \n 넣어서 하면 더 깔끔할 것 같아요!
javascript-lotto-precourse
기능 목록 - turtlehwan
중점 사항