[1단계 - 행운의 로또 미션] 위니(김예지) 미션 제출합니다#96
Conversation
Co-authored-by: rladpwl0512 <rladpwl0512@naver.com>
Co-authored-by: rladpwl0512 <rladpwl0512@naver.com>
Co-authored-by: rladpwl0512 <rladpwl0512@naver.com>
Co-authored-by: rladpwl0512 <rladpwl0512@naver.com>
Co-authored-by: rladpwl0512 <rladpwl0512@naver.com>
Co-authored-by: rladpwl0512 <rladpwl0512@naver.com>
Co-authored-by: rladpwl0512 <rladpwl0512@naver.com>
Co-authored-by: rladpwl0512 <rladpwl0512@naver.com>
Co-authored-by: rladpwl0512 <rladpwl0512@naver.com>
lsw1164
left a comment
lsw1164
left a comment
There was a problem hiding this comment.
안녕하세요 위니님, 문의해본 결과 리뷰를 단계별로 받아야되는 상황입니다.
현재 구현하신 부분은 백업해두시고 1단계 요구사항만 구현한 상태로 다시 리뷰 요청 부탁드립니다 🙏
rladpwl0512
force-pushed
the
rladpwl0512
branch
from
6667a84 to
d68299f
Compare
|
리뷰어님 안녕하세요~ +데모페이지 링크입니다. |
lsw1164
left a comment
lsw1164
left a comment
There was a problem hiding this comment.
안녕하세요 위니님, 구조가 간결하고 읽기 좋았습니다. 몇가지 코멘트 남겼으니 참고 부탁드릴게요 :)
| it('로또의 숫자는 중복되서는 안된다', () => { | ||
| const lottoModel = new LottoModel(); | ||
| lottoModel.setLottoCount('1000'); | ||
| lottoModel.setLottos([[1, 2, 3, 4, 5, 6]]); |
There was a problem hiding this comment.
중복된 케이스, 중복되지 않은 케이스 모두 검증해야 될 거 같습니다 :)
There was a problem hiding this comment.
현재 중복되지 않은 케이스만 작성했네요...
말씀해주신대로 중복된 케이스에 대해서도 테스트코드를 작성해보겠습니다~
| while (lottoNumberSet.size < LOTTO_NUMBERS.LOTTO_LENGTH) { | ||
| lottoNumberSet.add(getRandomNumber(LOTTO_NUMBERS.MIN_LOTTO_NUMBER, LOTTO_NUMBERS.MAX_LOTTO_NUMBER)); |
There was a problem hiding this comment.
무한루프는 생각하지 못했습니다...!
무한루프가 도는 상황이라고 하면, 계속해서 중복되는 숫자가 나와서 lottoNumberSet을 완성하지 못하고 반복문을 계속 도는 상황일까요? (사실 이런 상황은 가능성이 없다고 생각을 했고, 그래서 무한루프는 고려못했습니다)
There was a problem hiding this comment.
특별한 케이스가 아니라면 확률적으로 퍼포먼스가 달라지는 로직은 피하는 편입니다.
중복 없이 로또 번호를 추출할 때 꼭 번호를 뽑을때마다 이미 뽑았던 번호인지 확인해야될까요?
| const lottos = []; | ||
| for (let i = 0; i < this.getLottoCount(); i += 1) { | ||
| lottos.push(this.getLottoNumbers()); | ||
| } | ||
|
|
||
| return lottos; |
| this.lottos = []; | ||
| } | ||
|
|
||
| setLottoCount(inputPrice) { |
There was a problem hiding this comment.
setLottoCount라고 하니까 순수한 setter 같은 네이밍이라는 생각이듭니다.
(lottoCount를 파라미터로 받아서 그대로 lottoCount 값을 세팅할 거 같은)
하지만, inputPrice라는 파라미터를 받고 추가 로직을 수행하는군요.
그리고 두번째로, lottoCount를 상태로 둬야할까? 라는 생각을 해봤습니다.
'lottoCount 상태값'과 'lottos 상태값의 길이'랑 어떤 관계일까요?
There was a problem hiding this comment.
감사합니다~ set은 상태값을 그대로 바꿔주는 것을 기대하게 하는 함수명이라는 것을 배웠습니다!
그리고 말씀해주신 대로 lottoCount의 상태값과, lottos 상태값의 길이는 같습니다.
generateLottos함수에서 로또를 생성할 때, getLottoCount만큼 생성을 해주고 있습니다.
이부분은 조금 더 고민해보도록 하겠습니다!
| return this.lottoCount; | ||
| } | ||
|
|
||
| getLottoNumbers() { |
There was a problem hiding this comment.
getLottoNumbers라는 네이밍에 대해서도 다시 고민해보면 좋겠습니다.
새로 번호는 생성한다는 의도를 전달하기 위해 get보다 더 명확한 네이밍이 있을 거 같습니다.
There was a problem hiding this comment.
generateLottoNumers 와 같이 바꿔 보도록 하겠습니다~!!
어렵지만, 의도를 잘 드러낼 수 있는 이름을 작성하도록 꾸준히 노력해야겠네요!👏
|
|
||
| export default class LottoController { | ||
| constructor() { | ||
| this.model = new LottoModel(); |
There was a problem hiding this comment.
this.model 보다 사용하는 곳에서 바로 알 수 있도록 이름을 바꿔주면 어떨까요?
There was a problem hiding this comment.
코드를 읽는 사람이 이해하기 쉽도록, 가독성 있게 바꿔주도록 하겠습니다!
| this.$lottoPriceForm = document.querySelector('#lotto-price-form'); | ||
| this.$lottoPriceInput = document.querySelector('#lotto-price-input'); | ||
| this.$lottoPriceButton = document.querySelector('#lotto-price-button'); |
There was a problem hiding this comment.
이벤트 바인딩을 위해서 dom객체를 직접 들고 있는거라면
이벤트 등록하는 책임을 View로 옮긴다면 컨트롤러에서 dom객체에 직접 접근할 필요가 없겠군요?
There was a problem hiding this comment.
컨트롤러에서 dom에 직접 접근하지 않도록, 말씀해주신 방식을 참고하여 view에서 이를 처리해봐야겠네요~
이번 수업시간에서 구조짜는것에는 정답이 없다는 것을 배우고 구조에 대해 더 고민이 많이됩니다 😅
lsw1164
left a comment
lsw1164
left a comment
There was a problem hiding this comment.
위니님, 스텝2랑 히스토리가 꼬일 수 있어서 수정이 어려운 거 같습니다.
다음 스텝에서 뵙겠습니다!
3 participants
안녕하세요, 위니 미션 제출합니다.
이번에는 자스민👍과 페어를 진행했습니다.
사실... 실수를 하나 했는데요,
레벨1에 대한 기능요구사항을 보고 구현했어야했는데, 생각 못하고 figma 디자인을 보고 기능구현 목록을 작성했고, 그러다 보니 레벨2까지 모든 기능을 구현해버렸습니다. (알아챘을때는 너무 멀리와버려서, 결국 레벨2 모든 기능까지 구현했습니다... 죄송합니다 🥲)
레벨2 기능은 다 완성했지만, 제가 부족한 부분이 많기 때문에 남은 긴 시간동안 이번 미션에서 페어를 통해 배운 것, 추가적으로 필요한 것에 대한 공부도 많이 하고, 코드도 계속 개선해보도록 하겠습니다.
구조
docs/README.md에 첨부했습니다 :)데모페이지
https://rladpwl0512.github.io/javascript-lotto/
질문
MVC 패턴을 구현할 때 이벤트를 바인딩하는 함수를 controller, view 중 어디에 둘지 고민을 하다가, 결국 controller에 넣었습니다.
어떤것이 더 적절할지, 리뷰어님의 의견이 궁금합니다!
리뷰 잘 부탁드립니다! 감사합니다 :)