[1단계 - 행운의 로또 미션] 결(김윤진) 미션 제출합니다 by yunjin-kim · Pull Request #87 · woowacourse/javascript-lotto

yunjin-kim · 2022-02-24T02:07:29Z

안녕하세요 결이라고 합니다

mvc패턴을 사용해
전체 로직을 view, model, controller로 구분했습니다
view도 도메인별로 다시 한번 나누었습니다

코드를 페어와 함깨 짜다가 서로 의견은 달랐지만
정확히 어떤게 정답이다라고 판단하기 어려운 문제가 있어 답변 부탁드립니다

model, view의 인스턴스는 어디에서 생성하는 것이 좋을까요
현재 코드에서는 index.js에서 생성했지만 controller.js에서 생성하는 것과 고민했습니다
model에서 private 필드를 이렇게 선언하는것이 옳은 것인지 고민했습니다
메서드내의 임시변수 제거 vs 가독성 있는 변수 생성에 대해 고민했습니다
저는 임시변수를 제거할 수 있으면 최대한 제거하는 편이여서 의견이 상충했습니다
view이벤트, 로또구매나 토글 버튼의 이벤트 리스너를
controller vs view 어디에서 둥록하는 것이 옳은지 고민했습니다

감사합니다😀

데모페이지

Co-authored-by: SangMin Song <ssmin940606@gmail.com>

austinpark420 · 2022-02-24T14:34:13Z

안녕하세요 결!
이렇게 만나뵙게되서 반갑습니다!

MVC 패턴 관련해서는 제가 예전에 스터디를 하면서 다른 분에게 공유받았던 내용 전달드릴게요!

Model은 Controller와 View에 의존하지 않아야 한다.
→ Model 내부에 Controller와 View에 관련된 코드가 있으면 안 된다.
→ 즉 모델 내부에서 컨트롤러와 뷰에 있는 것을 임포트하면 안 됨
View는 Model에만 의존해야하고, Controller에 의존하면 안 됨.
→ View에는 Model의 코드만 있을 수 있고, Controller의 코드가 있으면 안 됨.
View가 Model로부터 데이터를 받을 때,
사용자마다 다르게 보여주어야 하는 데이터에 대해서만 Model에서 받야아 한다.
→ 공통으로 보여지는 부분은 View가 자체적으로 가지고 있어야하는 것.
Controller는 Model과 View에 의존해도 된다.
View가 Model로부터 데이터를 받을 때, 반드시 Controller에서 받아야 한다.
→ 데이터를 받을 때 Controller 코드 안에서만 받야 한다.

Q: model, view의 인스턴스는 어디에서 생성하는 것이 좋을까요
현재 코드에서는 index.js에서 생성했지만 controller.js에서 생성하는 것과 고민했습니다
A: controller.js 에서 인스턴스를 생성하면 index.js가 깔끔해지겠죠. 저는 개인적으로 controller.js 생성하는 편입니다!

Q:model에서 private 필드를 이렇게 선언하는것이 옳은 것인지 고민했습니다
A: 모델에서만 사용하는 부분에 대해서 비교적 잘 작성해주신 것 같습니다!

Q: 메서드내의 임시변수 제거 vs 가독성 있는 변수 생성에 대해 고민했습니다
저는 임시변수를 제거할 수 있으면 최대한 제거하는 편이여서 의견이 상충했습니다
A: 이 부분이 참 애매한 부분인데요. 저는 개인적으로 다른 사람이 봤을 때, 임시변수로 치환할지 고민되는 내용을 남이 봤을 때 유추가 가능한지 아닌지로 판단합니다!

Q:view이벤트, 로또구매나 토글 버튼의 이벤트 리스너를
controller vs view 어디에서 둥록하는 것이 옳은지 고민했습니다
A: DOM에 대한 이벤트 바인딩은 view에서 담당하고 그 이벤트에대한 액션은 controller에서 하면 역할이 명확할 것 같습니다!

제 개인적인 생각이기 때문에(저도 아직 부족해서 정답은 아닙니다!) 결은 어떻게 생각할지 궁금하네요!

austinpark420 · 2022-02-24T14:36:18Z

구현상에 2가지 이슈를 발견해서 전달 드립니다.

로또 구입을 새로했을 경우, 기존 로또구매한 내역이 사라지지않고 더해집니다!
로또 구매 수가 10개가 넘어가게되면 토글버튼이 없어집니다!

austinpark420

안녕하세요 결!!
이렇게 리뷰를 하게되서 정말 반갑습니다.
리뷰 관련해서 코멘트를 남겨놨습니다!
files changed에 남기기 어려운 리뷰는 conversation부분에 남겨놓았으니 확인 부탁드릴게요!

고생 많으셨습니다 😄

austinpark420 · 2022-02-24T06:23:12Z

+  font-weight: bold;
+  letter-spacing: 1.25px;
+  margin-left: 15px;
+


공백을 둔 이유가 있을까요?ㅎㅎ

체크를 못했네요ㅜㅜ
앞으론 더 꼼꼼하게 체크하겠습니다!

austinpark420 · 2022-02-24T06:24:19Z

@@ -0,0 +1,132 @@
+html,


user agent sylesheet 고려하셔서 reset.css 파일을 생성하셨나 보네요 👍

austinpark420 · 2022-02-24T06:31:20Z

-  it("테스트 명세", () => {
-    expect(true).toBe(true);
-  });
+import {


줄바꿈이 일정하지 않는 것 같습니다. 규칙을 만들어서 동일하게 적용해보시면 어떨까요?ㅎ

eslint 설정 바꾸면 수정을 깜빡했네요
수정완료입니다!

austinpark420 · 2022-02-24T12:48:25Z

+  - [x] 금액은 빈값으로 입력할 수 없다
+  - [x] 금액은 음수를 입력할 수 없다
+- [x] 구입한 로또 금액만큼 로또를 발급할 수 있어야 한다
+  - [] 발급 후 로또의 번호를 볼 수 없다


발급 후 로또의 번호를 볼 수 없다, 번호 보기 토글 버튼을 클릭해 로또 번호를 볼 수 있어야 한다. 구현하신 것 같은데 체크가 안되어 있네요ㅎ

수정완료입니다!

austinpark420 · 2022-02-24T12:56:26Z

@@ -0,0 +1,10 @@
+export const LOTTO = Object.freeze({


LOTTO만 Object.freeze을 한 이유가 있을까요?ㅎ

이 객체는 변경할 이유가 없는 값이기 때문에 불변객체로 만들었습니다
에러 메시지를 담은 객체에도 하면 좋을 것 같네요!

austinpark420 · 2022-02-24T13:10:24Z

+    return lottoNum;
+  }
+
+  #generateRandomNum() {


generateRandomNum함수가 LottoModel에서 가지고 있는 이유가 궁금합니다.ㅎ

흠...generateRandomNum 메서드가 LottoModel에서 가지고 있는 이유는 로또 번호를 하나씩 반환하니까 로또 관련 데이터라고 생각해서 lottoModel에 있어야 한다고 생각했습니다
오스틴은 따로 함수로 빼는 것이 좋다고 생각하시나요?
오스틴의 의견이 궁금합니다!

저는 단순히 generateRandomNum를 구하는 함수는 유틸에 가깝다고 생각했거든요.
반환된 숫자 자체는 LottoModel 모델이 가져가야하는 부분은 맞지만요ㅎ
이 부분은 관점차이인것 같아서 결의 생각이 궁금했습니다 ㅎ

austinpark420 · 2022-02-24T13:19:37Z

+  expect(lottoResult).toHaveLength(lottoCount);
+  expect(isCorrectLottoLength).toBe(true);
+})
+


토글도 구현했으니 그 부분에 대한 test 코드도 있으면 좋을 것 같습니다!

토글은 css만 조작해서 유닛 테스트보다는 e2e테스트에 가깝다고 생각해서 구현하지 않았습니다
오스틴은 어떻게 생각하시나요?

네 제가 잘못생각했네요 🙏
e2e테스트에서 적용해주시면 될것 같습니다 ㅎ

austinpark420 · 2022-02-24T13:21:07Z

+
+  submitPurchaseLotto(event) {
+    const purchaseMoney = event.detail;
+    if (!isValidPurchaseMoney(purchaseMoney)) {


잘못된 금액이기 때문에 input value 값은 날려도 괜찮을 것 같습니다! ux측면에서 어떻게 생각하세요?ㅎ

좋은 것 같습니다!
수정완료입니다

yunjin-kim · 2022-02-24T16:16:52Z

안녕하세요 오스틴, 소중한 리뷰 감사드립니다!
리뷰 해주신 부분 중 조금 의문이 드는 부분이 있어서 답변 부탁드립니다😀

질문 사항

View는 Model에만 의존해야하고, Controller에 의존하면 안 됨.
→ View에는 Model의 코드만 있을 수 있고, Controller의 코드가 있으면 안 됨.
view에 model 코드가 있다면 view에서 직접적으로 model의 데이터를 controller 거치지 않고 취득할 수 있다는 뜻인가요?
View가 Model로부터 데이터를 받을 때, 반드시 Controller에서 받아야 한다.
→ 데이터를 받을 때 Controller 코드 안에서만 받야 한다.
제가 이해한 바로는 이 부분과 상충하는 것 같습니다
사용자마다 다르게 보여주어야 하는 데이터에 대해서만 Model에서 받야아 한다.
→ 공통으로 보여지는 부분은 View가 자체적으로 가지고 있어야하는 것.
만약 예를 주식 서비스로 든다면
삼성전자의 주가 데이터는 view에서 관리하고
유저의 데이터는 model에서 관리한다는 뜻으로 이해하면 될까요?

이슈 관련

로또 구입을 새로했을 경우, 기존 로또구매한 내역이 사라지지않고 더해집니다!
로또를 새로 구입하면 기존 로또 구매 내역은 초기화하도록 구현했습니다
로또 구매 수가 10개가 넘어가게되면 토글버튼이 없어집니다!
로또 구매 개수를 최대 5개로 정했습니다
5개로 정한 이유는 로또 한장에 천원부터 오천원까지 구매할 수 있어서 이렇게 결정했습니다

감사합니다👍
데모페이지

austinpark420 · 2022-02-24T23:43:52Z

Q: 제가 이해한 바로는 이 부분과 상충하는 것 같습니다
A: 제 설명이 조금 부족하게 설명드린 것 같네요ㅠ
MVC 패턴 관련한 질문에 답을 드리면 Controller가 Model의 데이터를 업데이트하고 그 업데이트된 데이터를 View가 그리는 디자인 패턴인데요.
제가 전달드린 내용을 좀더 자세하게 설명드리면 아래와 같습니다.

View에는 Model의 코드만 있을 수 있고, Controller의 코드가 있으면 안 됨.
-> Controller를 통해 Model의 데이터가 업데이트되기때문에 Controller의 코드가 있을 필요가 없다.
View가 Model로부터 데이터를 받을 때, 반드시 Controller에서 받아야 한다.
-> Controller가 Model을 업데이트하고 그 업데이트된 Model의 데이터를 받아야한다. 위 글에서 "반드시 Controller에서 받아야 한다."는 Controller의 액션의 통해서 받아야한다 라고 말씀드리는게 명확했던 것 같네요. 🙏

Q: 사용자마다 다르게 보여주어야 하는 데이터에 대해서만 Model에서 받야아 한다.
-> 주식 서비스는 예시는 조금 복잡한 것 같습니다... 주식 데이터, 유저 데이터 모두 데이터이기때문에 다른 Model으로 관리되고 있을 것 같네요. 이 부분은 좀 더 생각해보시면 될 것같습니다. 지난 자동차 경주미션을 예로 들면 자동차 이름, 주행거리는 Model에서 관리하고 자동차이름을 표시하는 박스, 화살표 모양, 스피너 등은 View에서 관리하는 것이라고 생각하시면 될 것 같습니다!!

추가로 roy-jung 님이 록바님께 쓴 리뷰 중에 MVC 패턴에 대해서라는 글을 읽어보시면 좋을 것 같습니다!!
나머지 부분은 먼저 리뷰요청주신 크루님 리뷰를 끝난 뒤에 확인해볼게요 🙏

yunjin-kim · 2022-02-25T05:13:03Z

안녕하세요 오스틴
오늘 오전에 강의를 듣고 난 후 제가 문제 해결을 고민하지 않고 해결책을 먼저 도입한 것을 알게 되었습니다
그래서 처음 구조부터 다시 짜보려고 합니다
오스틴에게 죄송하지만 리뷰 다시 부탁드립니다🥲
다시 짜고 리뷰 요청 드리겠습니다!

austinpark420 · 2022-02-25T05:26:37Z

안녕하세요 결! 구조를 처음부터 다시 짜신다니 쉽진않은 결정이셨을텐데 대단하신 것 같아요. 👍
다만 기한내에 요구사항을 구현해내는 능력도 중요한 능력인이라고 생각합니다.
제가 주말에 개인적인 사정이 있어서 리뷰하기는 좀 어려울 것 같습니다 ㅠㅠ...
구조 변경해주신 부분을 step2에 리뷰요청때 같이 확인해봐도 괜찮을까요?
새로 짜는 구조가 맞는지 불안하실 수도 있을거라고 생각하는데요.
지금 문제를 접근하는 방법이 달라서 그 부분은 크게 신경을 안쓰셔도 될 것같아서 말씀드립니다. 😄

austinpark420 · 2022-02-25T06:28:44Z

제가 조금 더 생각해 봤는데 지금 구조에 대해서만 너무 많은 고민을 하고계신듯 해요.
디자인패턴의 의미는 설계 문제에 대한 해답을 문서화하기 위해 고안된 형식 방법입니다. 그만큼 견고하게 만들어졌다고 할 수 있죠.
MVC패턴도 그 중에 하나고요.
접근법은 다를수도 있지만 지금 구조가 나쁘지는 않다고 생각합니다.
지금 구조를 다시짜는 것도 충분히 도움이 되시겠지만 다음 미션을 수행하실 때 다른 접근법으로 구현해보시는 것도 좋을 것 같아요.
아직 해야할 미션도 많이 남아있잖아요 😄
이번 미션에서는 구조 외에 UX/UI, 기능 구현 방법등 다양한 부분에대해서 함께 고민하시는 걸 추천드려요 👍
선택은 결이 하시면 되고요! 제 의견에 대해서 말씀드렸습니다 😄

yunjin-kim · 2022-02-25T07:13:39Z

오스틴의 의견 너무 감사합니다
코드를 처음부터 다시 짜는 것은 개인적으로 하는게 맞는 것 같네요
일단 코드의 구조는 이대로 가면서 리펙터링 하는 부분으로 진행하겠습니다
혼란 드려서 죄송하네요🥲
(사실 이미 다 엎고 다시 짜긴 했습니닼ㅋㅋ)

austinpark420

step1 구현하시느라 고생많으셨습니다!
서로 이야기한 부분에 있어서는 step2에서 코멘트 남기도록 하겠습니다!
그럼 주말 잘보내세요! 👍

austinpark420 · 2022-02-25T07:04:02Z

+  expect(lottoResult).toHaveLength(lottoCount);
+  expect(isCorrectLottoLength).toBe(true);
+})
+


네 제가 잘못생각했네요 🙏
e2e테스트에서 적용해주시면 될것 같습니다 ㅎ

austinpark420 · 2022-02-25T07:05:30Z

+    return lottoNum;
+  }
+
+  #generateRandomNum() {


저는 단순히 generateRandomNum를 구하는 함수는 유틸에 가깝다고 생각했거든요.
반환된 숫자 자체는 LottoModel 모델이 가져가야하는 부분은 맞지만요ㅎ
이 부분은 관점차이인것 같아서 결의 생각이 궁금했습니다 ㅎ

yunjin-kim and others added 17 commits February 22, 2022 14:46

docs: 기능명세 작성

504e883

Co-authored-by: SangMin Song <ssmin940606@gmail.com>

chore: eslint, prettier 설정

01c5285

Co-authored-by: SangMin Song <ssmin940606@gmail.com>

test: 금액 유효성검사

f39d23c

Co-authored-by: SangMin Song <ssmin940606@gmail.com>

chore: eslint 설정변경

6d0ae4e

Co-authored-by: SangMin Song <ssmin940606@gmail.com>

design: 기본UI 작성

228a754

Co-authored-by: SangMin Song <ssmin940606@gmail.com>

feat: 로또 구입 금액 입력 기능 추가

c606d30

Co-authored-by: SangMin Song <ssmin940606@gmail.com>

test: 구입한 로또 금액만큼 로또 개수 확인

86eaa23

Co-authored-by: SangMin Song <ssmin940606@gmail.com>

feat: 로또 발급 기능 추가

fbae184

Co-authored-by: SangMin Song <ssmin940606@gmail.com>

feat: 번호 보기 토글버튼 기능 추가

61d3de9

Co-authored-by: SangMin Song <ssmin940606@gmail.com>

test: 구입한 로또 금액만큼 개수 확인 테스트 수정

cda0d60

Co-authored-by: SangMin Song <ssmin940606@gmail.com>

design: 로또 style 수정

46640fc

Co-authored-by: SangMin Song <ssmin940606@gmail.com>

refactor: 상수화

b9dace2

Co-authored-by: SangMin Song <ssmin940606@gmail.com>

chore: eslint 수정 및 eslint에 맞게 코드 수정

6e7bc05

chore: 웹팩 배포 설정 변경

cac4318

refactor: html에서 불러오는 js,css 삭제

a2b397b

refactor: model private 필드 선언 변경

5305fa0

chore: github-page 배포 설정

737d3a5

pocojang requested a review from austinpark420 February 24, 2022 02:36

pocojang assigned austinpark420 Feb 24, 2022

austinpark420 suggested changes Feb 24, 2022

View reviewed changes

yunjin-kim added 5 commits February 25, 2022 00:27

fix: 코드 규격에 맞게 수정

9a03f6e

feat: 로또 구매 잘못된 값 입력시 input값 제거

fd27369

docs: README 수정

8abac22

feat: 로또 최대 구매가능 개수 기능 추가

22b929e

feat: 로또 재구매시 전에 샀던 기록 초기화 기능 추가

d8646bf

austinpark420 approved these changes Feb 25, 2022

View reviewed changes

austinpark420 merged commit b5c522e into woowacourse:yunjin-kim Feb 25, 2022

Conversation

yunjin-kim commented Feb 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

austinpark420 commented Feb 24, 2022

Uh oh!

austinpark420 commented Feb 24, 2022

Uh oh!

austinpark420 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yunjin-kim Feb 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yunjin-kim Feb 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yunjin-kim commented Feb 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

austinpark420 commented Feb 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yunjin-kim commented Feb 25, 2022

Uh oh!

austinpark420 commented Feb 25, 2022

Uh oh!

austinpark420 commented Feb 25, 2022

Uh oh!

yunjin-kim commented Feb 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

austinpark420 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

yunjin-kim commented Feb 24, 2022 •

edited

Loading

yunjin-kim Feb 24, 2022 •

edited

Loading

yunjin-kim Feb 24, 2022 •

edited

Loading

yunjin-kim commented Feb 24, 2022 •

edited

Loading

austinpark420 commented Feb 24, 2022 •

edited

Loading

yunjin-kim commented Feb 25, 2022 •

edited

Loading