-
Notifications
You must be signed in to change notification settings - Fork 168
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
[2단계 - 웹 기반 로또 게임] 초코(강다빈) 미션 제출합니다. #327
Conversation
- 기존의 로또 컨트롤러는 readline 을 불러오기 때문에 에러가 발생해 ui에서 입력받은 값을 매개변수로 넘겨주는 수정 - step2-index.js에 step2-LottoController.js 연결
1. 구입금액폼 제출로 변경 2. css 확인을 위해 직접 넣어놨던 로또 데이터를 실제 발행되는 로또 데이터로 변경 3. lottoSection을 display:none에서 block으로 변경하여 보여주기
1. 로또 티켓 내의 숫자 하나씩 요소로 지정 2. 로또 티켓 내의 숫자 요소 스타일링 3. 전체 스타일 flex로 변경
1. ui 구현 2. 당첨번호, 보너스 번호 기능 컨트롤러 연결
1. 문자열+숫자 조합이 들어올 경우 추가 2. 보너스 번호, 당첨 번호 입력 오류 출력할 영역 winningAndBonusError 추가 3. 입력 관련 메서드의 eslint(max-params) 오류로 인해 {} 로 묶는 수정
- 로또 발행 단계까지의 메서드 정리 완료
1. 당첨번호와 보너스 번호를 정상 입력했을 경우에만 결과 확인하기 모달창 나타나도록 수정 2. 당첨번호 입력 메서드 분리 3. 보너스번호 입력 메서드 분리 4. 모달창의 수익률 출력 메시지는 OUTPUT_MESSAGES 의 문구 활용
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.
안녕하세요 초코~!
두번째 스텝까지 고생많으셨습니다!
지난 스텝에서 만들어둔 도메인 함수들을 적절히 호출해 구현하신 것이 아주 좋네요👍
도메인과 ui의 분리를 해야한다면 어떻게 해야하는 걸까요?
도메인과 UI에 대한 개념이 아직 완벽하게 와닿지는 않으시군요!
조금 표현을 바꿔 볼까요?
프로그램은 보통 문제를 해결하기 위해 만들어집니다. 도메인은 문제를 해결하는 솔루션이고 UI는 그 솔루션을 이용하는 방법입니다.
로또 게임도 프로그램이기에 문제를 정의하고 있어요.
- 일정 금액만큼 랜덤한 로또를 구매하고 싶어.
- 실제 당첨 번호와 비교하여 얼만큼 수익을 냈는지 알고 싶어.
이 문제들을 해결하기 위해서는 다음과 같은 솔루션이 필요합니다.
- 주어진 금액으로 몇 장의 로또를 구매할 수 있는지 계산한다.
- 1~45사이의 랜덤한 숫자 6개를 뽑아 로또 하나를 발급한다.
- 위 방식으로 구매 가능한 수만큼 로또를 발급한다.
- 당첨 번호와 보너스 번호에 로또 번호들을 맞춰본 후 상금을 결정한다.
- 각 상금을 더한다.
- 구매한 금액 대비 얼마나 수익이 있었는지 계산한다.
솔루션만 있으면 될까요? 해결방식만 있고 실제로 문제를 해결해주지 못한다면 프로그램을 만드는 게 크게 의미가 없을 겁니다. 그래서 내가(User가) 이 솔루션을 '어떻게 이용할 건지' 방법을 제공해야해요.
말 그대로 UI, User Interface가 필요합니다.
- 돈을 어떻게 받을 것인지
- 발급한 로또 번호는 어떻게 보여줄 건지
- 당첨 번호는 어떻게 전달 받고
- 결과는 어떻게 알려줄건지
- 금액이 충분하지 않다거나, 당첨번호가 유효하지 않을 때는 어떤 방식으로 알려줄건지
이런 내용들이 UI의 관심사에요.
도메인과 UI를 어떻게 어떻게 분리해야할지 질문주셨는데 이렇게 놓고 보니 애초에 서로 관심사가 많이 다른 것 같지 않나요? 사실 도메인과 UI를 분리하자는 말의 핵심이 여기 있다고 생각해요. 관심사가 다른 친구들끼리 서로 참견하게 하지 말자는 겁니다. UI가 로또 번호를 웹으로 보여주든, 터미널에 보여주든, 심지어 그냥 초코가 직접 종이에 적어준다 하더라도 도메인 크게 신경쓰지 않아요. 어떤 방식으로든 금액이 주어지면 로또 번호를 뽑고 당첨 번호가 주어지면 결과를 낼 뿐입니다. 반대로 도메인이 뭐 대단한 알고리즘으로 완벽하게 로또 결과를 계산하건 말건 UI는 1도 관심이 없어요. 도메인이 결과를 알려주면 그걸 사용자에게 전달할 뿐이에요. 도메인과 UI가 철저히 자기 것에만 관심을 두고 불필요한 소통을 줄일 수록 분리는 자연스럽게 이루어집니다.
이렇게 서로 관심사가 명확하게 분리되면 자연스럽게 프로그램이 유연해지고 안전해 집니다. 조금 극단적이긴 했지만 cli => 웹으로 전환할 때 어떠셨나요? 이전 스텝에서 도메인과 UI가 잘 분리되어 있었다면 로또 번호를 어떻게 뽑는지, 결과를 어떻게 내는지 생각할 필요도 없었을 겁니다.(아마 초코가 그랬을 것 같네요!) 잘 분리되지 않았다면 UI 한 번 고치려다 도메인 로직 여기저기도 함께 수정하느라 꽤나 고생했겠죠..! 그만큼 도메인의 기존 동작도 보장이 어려웠을 거구요.
이야기 하다보니 주절주절 말이 좀 길어졌는데.. 이번 미션에서의 도메인과 UI의 관심사 항목을 쭉 나열해보시고 서로의 영역을 침범하지는 않았는지 + 그 때문에 UI 마이그레이션 과정에서 어려움은 없었는지 회고해보시면 더욱 도움이 되실 것 같습니다.
다시 시작하기 버튼을 눌렀을 때 게임이 재시작되는 로직을 페이지 새로고침으로 구현했는데, 수정이 필요할까요? 브콜의 의견이 궁금합니다
지금은 앱의 규모가 작으니 새로고침도 그리 나쁜 방법은 아닌 것 같기도 하네요.
다만 새로고침하면서 의도하지 않은 상태까지 초기화 되지는 않는지만 점검해보시면 좋겠습니다.
...그러면 내가 사용자의 입장이 되어 인사이트를 얻고 개선해나가야 하는 영역일까요? 아니면 UX 관련 인사이트를 얻을 수 있는 사이트가 있을까요?
사용자 입장에서 인사이트를 얻는 방법도 좋은데 어떤 게 좋은 UX인지에 대한 기준을 얻기에는 책이나 강의, 아티클 같이 정리된 자료들의 도움을 받는 것도 좋아보여요.
라고 했지만 당장 떠오르는 좋은 사이트나 자료는 없네요..! 슬랙에 질문을 남겨보면 어떨까요?
처음에 저는 html로 구조를 다 잡아두고, css를 적용하고, js를 추가하는 방향으로 했는데, 필요한 부분만큼 구조를 잡고, js를 추가하는 방식이 더 많은 것 같더라고요? ... 혹은 어떤 방식을 추천하시는지 궁금합니다!
주로 기능 단위로 작업합니다. 말씀하신 방법 중에는 후자가 되겠네요.
지금은 편하신 방식으로 작업하시면 될 것 같아요. 미션 요구사항이 더 복잡해지고 기존 방식에서 불편함이 느껴지면 그 때 다시 고민해보시죠.
index.html
Outdated
<!-- section 1 --> | ||
<div id="mainTitleSection" class="section-div title">🎱 내 번호 당첨 확인 🎱</div> | ||
|
||
<!-- section 2 --> |
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.
주석으로 section1,2,3,.. 이렇게 구분하기보다 각 영역을 section tag으로 잘 감싸주면 어떨까요?
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.
불필요한 주석은 줄여야겠어요! 그런데 id로 ~~Section이라고 표시를 해둬서 section tag를 추가할 필요는 없지 않을까 생각합니다.
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.
section div로 표현할 정도라면 이 태그의 이름을 다시 한 번 고민해보셔도 좋겠네요~
<section />
으로 표현할 수 있는데 <div class="section-div" />
로 표현할 이유가 있나요?
src/step2-index.js
Outdated
|
||
const lottoController = new LottoController(); | ||
|
||
window.onload = function () { |
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.
onload 시점에 실행해야하는 이유가 있나요?
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.
생각해 보지 않고 그냥 사용했었는데,
페이지를 로드하는 방법으로 크게 두가지가 있다는 것을 덕분에 알게 되었어요!
window.onload
: 페이지에 포함된 모든 리소스(이미지, 스타일시트, 스크립트 파일 등)가 로드되어야 실행됨, 많은 리소스가 있을 경우 상당한 시간이 소요될 가능성이 있음DOMContentLoaded
: HTML이 완전히 로드되고 DOM 트리가 완성되었을 때 발생됨- 이 때문에 DOMContentLoaded 이벤트는 window.onload보다 일반적으로 더 빠르게 실행됨
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.
굳굳👍
근데 제 의도는 정말 onload든 DOMContentLoaded든 이벤트 시점을 잡아야 겠다고 생각한 이유가 궁금했던 것이긴 했어요~!
script의 위치를 조정한다거나, script의 defer 어트리뷰트를 사용한다거나 하는 방식도 있을 것 같아서요!
src/step2-index.js
Outdated
let lottos = []; | ||
|
||
purchaseForm.addEventListener('submit', async function (event) { | ||
event.preventDefault(); | ||
const purchaseAmount = document.getElementById('purchaseInputField').value; | ||
lottos = await lottoController.purchaseLottos(purchaseAmount); |
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.
lottos를 여기서 관리해야하나요? LottoMachine 안에서 관리하는 게 좋아보여요.
lottos는 이 앱의 핵심 데이터 중 하나 같은데 이렇게 무방비하게 전역 변수로 두지 않았으면 좋겠어요
src/step2-index.js
Outdated
|
||
purchaseForm.addEventListener('submit', async function (event) { | ||
event.preventDefault(); | ||
const purchaseAmount = document.getElementById('purchaseInputField').value; |
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.
event 객체를 활용해 input value에 접근할 수는 없을까요?
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.
event 객체로 접근한다는 게 뭘까 고민했었는데 브콜의 다른 리뷰를 통해 알 수 있었어요!
document.getElementById('purchaseInputField').value
이 아니라
event.target.elements.purchaseInputField.value
로 접근하길 원하신 거겠죠?
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.validateIsInteger(purchaseAmount); | ||
this.validateIsAtLeast(purchaseAmount, OPTIONS.LOTTO.price); | ||
this.validateIsNumber(purchaseAmount); | ||
const numberInput = parseInt(purchaseAmount, 10); |
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인데 값을 가공하는 일까지 맡아서 하고 있군요..!
함수 이름만으로 예측할 수 있는 한 가지 일만 하도록 작성해주세요
안녕하세요 브콜, 다른 코멘트들은 반영했는데 이 코멘트에서부터 고민이 시작되어 코드 전체적인 수정을 고려하고 있어요. |
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.
안녕하세요 초코! 사실 이번 리뷰만 하고 머지할 생각이었는데 리팩토링 의지가 강하신 듯 하여 우선 RequestChanges로 남겨두도록 할게요.
제 의도에 맞게 잘 반영해주셔서 감사하고 추가 코멘트 하나 남겼으니 확인 부탁드려요.
리팩터링 방향에 대해 간단한 힌트만 하나 드리자면..
controller같은 애매한 이름보다는 차라리 UI라던가 View라는 이름에 모듈 '하나' 만 두고 철저히 UI로써 역할 하도록 작성해보시면 좋을 것 같아요.
여기서 UI로써의 역할은 '유저에게 입력을 받는 것'과 '유저에게 보여주는 것'이 메인입니다.
폼 입력을 받아서(onSubmit) => 도메인의 함수를 호출 => DOM 조작 하는 흐름이 주된 역할이 되도록! 진행하시다 어려움 있으시면 편히 질문 주셔요
static validate(purchaseAmount) { | ||
this.validateIsNumber(purchaseAmount); | ||
const numberInput = parseInt(purchaseAmount, 10); | ||
const numberInput = this.convertToNumber(purchaseAmount); | ||
this.validateIsInteger(numberInput); | ||
this.validateIsAtLeast(numberInput, OPTIONS.LOTTO.price); | ||
return numberInput; | ||
} |
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.
사실 const numberInput = this.convertToNumber(..)
이 부분이 아예 validate 밖으로 나가길 기대했어요.
validate이라는 함수를 호출할 때 저라면 순수하게 validate을 해주는 역할만 기대할 것 같습니다.
값이 유효하지 않을 때 에러를 던져주거나 값이 유효한지 아닌지 boolean을 반환하는 동작을 기대할 것 같아요.
내가 넣은 값을 어떤 방식으로는 parse
해서 반환해주는 동작을 기대할 것 같지는 않아요.
제가 말씀드린 예측 가능성이 여기서 나옵니다. 물론 이벤트 핸들러 같은 아이들까지 모든 동작을 함수 이름에 명시하기는 어렵겠지만 validate같이 작은 아이들 만큼은 그 이름으로 예측할 수 있는 일만 하도록 작성해주세요!
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.
PR머지하겠습니다! 고생 많으셨어요!
안녕하세요. 브콜(오태은), 초코입니다
이번 미션은 터미널 기반 게임을 UI 기반 게임으로 리팩토링하는 것이었는데요.
어떻게 연결을 해야 하는건가, 어떤 코드 구조/흐름을 가져야 하는 걸까 고민이 많이 되서 다른 크루들보다 미션 시작을 늦게한 것 같습니다.
이번 미션 리뷰도 잘 부탁드립니다 :)
🔗 배포링크
행운의 로또
🖼️ 실행화면
1️⃣ 2단계 - UI 기반 로또 게임
학습 목표
기능 요구사항
프로그래밍 요구사항
❓ 리뷰어님께 드리는 질문
💻 코드 부분
❤️ 개인 부분