[1단계 - 로또 미션] 도비(김정혁) 미션 제출합니다.#5
Conversation
- 로또 구매 결과에 대한 안내 메시지 기능 변경
- 로또 1장의 가격은 1,000원이다
- 로또 번호는 1부터 45까지의 숫자 중 중복되지 않은 6개의 숫자로 이루어져있다.
zereight
changed the title
[아키텍처] Component-Based-Architecture - 10내용
링크[렌더링] Reflow vs Repaint - 8내용
링크
[클래스] static 사용 - 7내용
링크 |
- setState: 상태 업데이트 방법도 e.target.checked로 판단
- clearInput
- 1단계에선 렌더링 로직에서 무조건 hidden 클래스를 없애면 됐음 - 하지만 2단계는 재시작 버튼이 존재함으로 lotto의 유무에 따라 hidden 클래스를 추가했다 지우는 로직 필요
There was a problem hiding this comment.
전체적으로 코드도 깔끔하고 역할 분리에도 신경을 쓰신듯 하여 읽기 쉬운편이었습니다.
리액트를 써보신듯한데 비슷한 패턴으로 구현하려고 노력 하신것 같습니다.
모델에 로또 발행기 같은 역할이 하나 더 추가 되면 더 명확해질 것 같습니다.
로또 발행기는 무엇을 알고 있어야 하는지(어떤 책임을 갖는 역할인지)를 중점적으로 고민해보시면 좋을 것 같습니다.
질답
DOM 객체를 가져올일이 이리저리 많은경우, 유지보수를 위해서 각각의 태그에 id 를 많이 쓰면 좋은 것인가요? 태그에 id를 넣는 일이 많아지니까 id를 남용하는 것 같아서 고민됩니다!
-> 페이지의 유일한 엘리먼트라면 id를 하시면 되고 비슷한 성격의 것들 이라면 data-* 로 하시면 좋습니다. data-section=lottoContainer, data-section=amountContainer 같은 식으로 하시면 각 섹션을 한 번에 가져올 수도, 가져온것을 세부적으로 구분하기도 쉽습니다.
class는 스타일을 위한 것이므로 class 로 여러개를 가져와서 조작하는것은 깨지기 쉽습니다. (스타일이 바뀌어 html 에서 클래스 이름을 바꿨는데 기능이 깨질 수 있는...)
컴포넌트 기반 아키텍처로 프로그래밍을 했는데요! 컴포넌트를 최대한 작게 나누는 것이 좋을까요? 1개의 컴포넌트가 담당하는 역할이 많아지니 가독성도 떨어지고 단일 책임의 원칙(?)에도 맞지 않는 것같아서 고민됩니다!
-> 네 최대한 작게 나누는것이 좋습니다. 컴포넌트 기반 아키텍쳐가 재사용 가능한 컴포넌트로 만들어 가는 것이니까요. 재사용 가능하려면 결국 단일 책임원칙이 중요하게 작용합니다.
index.html
Outdated
| <div class="w-100"> | ||
| <h1 class="text-center">🎱 행운의 로또</h1> | ||
| <form class="mt-5"> | ||
| <form id="lotto-perchase-input-container" class="mt-5"> |
There was a problem hiding this comment.
lotto-perchase-input-container 아이디는 추가하신걸 보면 처리 할려고 하셨던 것인지는 모르겠으나
인풋에 구입 금액을 입력하고 엔터를 치면 새로고침이 되네요.
form 이벤트 preventDefault() 를 해주는 부분도 있으면 좋을 것 같습니다.
로또 미션의 요구사항과 크게 관련은 없지만 프론트개발 실무의 기본이긴 하니까 기왕이면 처리 되면 좋을 것 같습니다.
src/js/utils/lotto.js
Outdated
|
|
||
| export const mod = (a, b) => a % b; | ||
|
|
||
| export const generateLottoNumbers = () => { |
There was a problem hiding this comment.
lotto.js 는 유틸이라고 보기 어려운 것 같습니다.
generateRandomNumber, divide, mod 는 숫자를 다루는 유틸이라는 하나의 역할로 묶을 수는 있으나 generateLottoNumbers 는 로또라는 특정 도메인의 문제를 해결하기 위함이므로 유틸로 보기 어렵고 isValidLottoNumbers 역시 마찬가지 입니다.
실세계에서의 상황을 생각해보시면 역할 분리에 좀 도움이 되실것 같습니다.
로또발행기한테 돈 넣고 로또 발행을 요청하면 로또를 반환하는 식으로 하셔서 책임을 나누면 어떨까 싶습니다.
There was a problem hiding this comment.
넵 로또 발행기 역할의 LottoManager를 만들어서 로또와 관련된 함수들을 가지도록 수정했습니다.
피드백 감사합니다!
src/js/components/App.js
Outdated
| this.lottoDisplay = new LottoDisplay({ lottos: this.lottos }); | ||
| } | ||
|
|
||
| createLottos(lottoCount) { |
There was a problem hiding this comment.
로또 App이긴 하지만 App 그 자체가 직접 로또를 발행(createLottos)하는것보다는 로또를 발행하는 모듈은 따로 있고 그걸 관리하고 뷰와 연결하고 하는 역할을 App 이 맡는게 좋을 것 같습니다.
요약하면 App 이 createLottos 를 하는것은 다소 과다한 책임을 맡고 있는 것 같습니다.
There was a problem hiding this comment.
네 정확한 피드백이십니다! 이 문제로 많은 고민을 했었는데요!
이번엔 옵저버 패턴과 Pub-Sub 패턴을 이용하여 최대한 App의 책임을 덜어주도록 수정했습니다!
- code depth를 줄여 가독성을 높이기 위함
- App.prototype.excute : state초기화 및 하위 컴포넌트 등록
- PubSub 구조를 활용하여 로또 상태 관리 - 로또 매니저는 로또 생성 및 검증에 대한 역할을 담당하도록 설계 - 로또 매니저 설계에 따른 테스트 내용 수정 Co-authored-by: zereight <zereight@users.noreply.github.com>
zereight
force-pushed
the
zereight-step1
branch
from
863b346 to
d457fe0
Compare
3 participants
안녕하세요. 리뷰어님!
이번에 리뷰요청 드리는 도비(김정혁) 입니다.
구현한 페이지는 실습 페이지 에서 확인하실 수 있습니다!
구조나 변수명, 성능 등의 다양한 관점에서 피드백 부탁드립니다. 감사합니다!
🎯 step1 구입 기능
alert질문 있어요!
id를 많이 쓰면 좋은 것인가요? 태그에id를 넣는 일이 많아지니까id를 남용하는 것 같아서 고민됩니다!