[2단계 - 웹 기반 로또 게임] 우디(류정우) 미션 제출합니다. #225
Conversation
src/view/view.js
Outdated
| const view = { | ||
| input, | ||
| convertVisibilityToHidden(...doms) { |
There was a problem hiding this comment.
파라미터에 ... 문법을 써주셨는데요.
어떤 의미로 쓰는 문법이며 어떤 이유로 적용하셨는지 설명 부탁드려요.
There was a problem hiding this comment.
...doms는 나머지 매개변수로, arguments를 배열로 만들어주기 위해서 사용했습니다.
해당 메서드는 호출시에 인자로 넘긴 dom에 대해 style의 visibility를 'hidden'으로 만들어주는 역할을 부여하려 했습니다
하지만 'hidden'으로 만들고싶은 dom이 여러개인 경우, 굳이 함수를 여러번 호출하지 않고, 호출시에 넘긴 모든 dom에 대해서 style의 visibility를 'hidden'으로 바꿔주도록 수정했습니다
arguments는 Array와 유사한 형태를 가지고 있지만 실제로는 Array가 아니라서 length를 제외한 Array 메서드를 사용할 수 없습니다.
때문에 나머지 매개변수로 만들어 넘겨받은 인자들이 담긴 Array로 바꿔주었습니다
[...doms].forEach((dom) => (dom.style.visibility = 'hidden'));위의 코드는 이미 Array인 상태에서 한번 더 전개연산자로 배열로 만들어 주고 있기 때문에 아래와 같이 수정했습니다
doms.forEach((dom) => (dom.style.visibility = 'hidden'));
src/view/view.js
Outdated
| }, ''); | ||
| }, | ||
|
|
||
| insertLottoCount($lottoCount, budget) { |
There was a problem hiding this comment.
전반적으로 insert라는 네이밍을 사용해주셨는데요.
이름만 보면 기존 내용에 추가할 거 같지만 실제 로직은 그냥 덮어쓰고 있어요.
의도를 더 잘 전달할 수 있도록 바꿔볼 수 있을까요?
src/step2-index.js
Outdated
| const $budgetForm = document.querySelector('.budget_form'); | ||
| const $budgetInput = document.querySelector('.budget_input'); | ||
| const $budgetError = document.querySelector('.budget_error'); | ||
|
|
||
| const $nextStepAfterBuyingLotto = document.querySelector('#next_step_buying_lotto'); | ||
|
|
||
| const $lottoCount = document.querySelector('.lotto_count'); | ||
| const $lottoList = document.querySelector('.lotto_list_box'); | ||
|
|
||
| const $lottoNumberForm = document.querySelector('.lotto_number_form'); | ||
| const $winningNumberInputs = document.querySelectorAll('.winning_number'); | ||
| const $bonusNumberInput = document.querySelector('.bonus_number'); | ||
| const $numberError = document.querySelector('.number_error'); | ||
|
|
||
| const $modal = document.querySelector('#modal'); | ||
| const $winningCounts = document.querySelectorAll('.winning_count'); | ||
| const $profitRate = document.querySelector('.profit_rate'); | ||
| const $modalCloseButton = document.querySelector('.modal_close'); | ||
| const $modalBackground = document.querySelector('.modal_background'); | ||
|
|
||
| const $retryButton = document.querySelector('.retry_button'); | ||
|
|
||
| let lottoGame; |
There was a problem hiding this comment.
한 파일에 굉장히 많은 변수가 있어서 코드 관리가 어렵겠다는 생각이 들었어요.
함수 내부에서 이 변수들을 사용하고 있는데요.
사용하는 변수가 상위 스코프에 선언된 변수인지 메서드 내부에서 선언한 변수인지 헷갈리네요.
수가 적으면 구분할 수 있을거 같아요. 분리할 수 없을까요?
There was a problem hiding this comment.
먼저 step2-index.js 파일에서는 이벤트를 잡는 역할만 담당하도록 수정했습니다.
eventHandler.js 파일은 잡은 이벤트에 대해 LottoGame과 소통하며 로직을 수행하도록 했어요
domHandler 폴더를 생성해서 연관있는 dom끼리 파일로 묶어 관리하도록 했어요
이 때, 반복되어 사용되는 로직을 domHandler/utils 파일에서 유틸 함수로 구현했습니다
그 외의 domHandler 하위의 파일에서는 실질적으로 dom의 수정과 value를 읽어오는 역할 즉, view의 역할을 담당하고 있어요
eventHandler에서 데이터를 받아 dom을 수정하라는 명령을 내렸는데 domHandler에서 또 view에 dom을 수정하라고 명령하는게 이상하다고 생각했습니다
때문에 view를 없애고 위와 같이 구현했어요
간략하게 도식화 해봤습니다😭
src/step2-index.js
Outdated
| }; | ||
|
|
||
| const renderModal = (winningNumbers, bonusNumber) => { | ||
| const winningStatus = [...lottoGame.getWinningStatus(winningNumbers, bonusNumber)].reverse(); |
There was a problem hiding this comment.
Array가 있는 거의 모든 곳에 [...] (Spread Syntax 문법)을 사용해주셨어요.
어떨 때 사용하는 게 좋을까요? Array면 무조건 써야 할까요?
There was a problem hiding this comment.
띠용..
이미 (불변성을 지켜서 return받은) 숫자 배열에 왜 또 전개 구문을 사용했는지 모르겠네요ㅠㅠ
Array면 무조건 쓰는건 굉장히 이상합니다
이미 Array인 것을 똑같이 Array로 만들 뿐이니까요
전개 구문을 사용하는 이유로는 대표적으로 아래와 같은 것들이 있습니다
복사
const arr = [1, 2, 3];
const arr2 = [...arr];
arr2.push(4);
console.log(arr); // [1, 2, 3]
console.log(arr2); // [1, 2, 3, 4]연결
let arr = [1, 2];
const arr2 = [3, 4];
arr = [...arr, ...arr2]; // [1, 2, 3, 4]분해
const arr = [3, 4, 5, 1, 2, 7];
const maxNumber = Math.max(...arr);
|
|
||
| const input = { | ||
| getWinningNumberInputsValues() { | ||
| return [...$winningNumberInputs].map((input) => Number(input.value)); |
There was a problem hiding this comment.
map을 쓸거라면 깊은 복사를 한 번 더 할 필요가 없었을 거 같아요.
| const DOM_TYPE = { | ||
| budget: $budgetError, | ||
| number: $numberError, | ||
| }; |
There was a problem hiding this comment.
error.js 파일이 모든 에러를 처리할까요?
타입이 늘어난다면 관리하기가 어려울 것 같습니다.
budget과 number를 한 파일에 같이 묶으셨는데 이 둘은 어떤 관련이 있을까요?
사용처에서는 string이 아니라 상수로 호출하는 게 좋을 거 같아요.
왜냐하면 받을 수 있는 형태가 딱 정해져 있기 떄문입니다. ("budget", "number")
그 정해진 형태를 외부에 제공하는 게 휴먼 에러 방지 차원에서 좋을 거 같아요.
| event.preventDefault(); | ||
| error.clearError('budget'); | ||
|
|
||
| const budget = event.target[0].value; |
There was a problem hiding this comment.
인덱스 넘버로 직접 접근하는 방식은 지양하는 편입니다.
인덱스 위치가 바뀔 때 관련 코드를 추적하기 힘들죠.
그리고 event.target이 배열이 되나요?
웹 기반 로또 게임
배포 링크
애플리케이션 실행 방법
사용자나 애플리케이션의 흐름을 위해 어떤 명령을 지시하는 것이 아닌, 사용자의 입력에 따라 로직을 수행하는 것이기 때문에 컨트롤러가 별도로 필요하지 않다고 생각해서 제거했습니다
step2-index
이벤트를 Listen하는 기능을 담당합니다eventHandler
잡은 이벤트에 대해
도메인 로직을 수행하는 역할을 담당합니다domHandler
dom에서 값을 읽어오거나dom을 조작하는 기능을 당담합니다질문 사항
HTML 태그안에 들어가는 문자열을 굳이 모두 상수화 할 필요가 있을까요?
Dom으로 조작할 필요가 없는 태그안의 문자열에 대해서 상수화를 진행하고 자바스크립트로
innerText를 사용해서 문자열을 넣어준다면사용자는 인터넷 환경에 따라 빈 화면만을 보게 될 가능성이 있다고 생각했습니다
리트 안녕하세요!
우선 도메인 로직을 수정하지 않고 구현해봤습니다
콘솔 기반 미션과 다르게 이번 미션은 생각보다 많이 헤매고, 어렵게 느껴졌어요
무엇보다 힘들었던 것은 모르는걸 모르겠다는 부분이였습니다😭
리트가 1단계 미션에서 질문을 많이 던져주신 덕분에 여러가지를 공부해볼 수 있었어요!
이번에도 잘 부탁드리겠습니다!!