[1단계 - 행운의 로또 미션] 지그(송지은) 미션 제출합니다. #23

zigsong · 2021-02-18T08:11:24Z

안녕하세요! 페어 티케와 로또 미션 진행한 지그입니다.
잘 부탁드립니다 😊

데모 - 지그의 로또 미션

✅ 1단계 미션 TODO

🎰 로또 🎰

1번부터 45번까지의 공 중에서 6개를 추첨하고, 이어서 보너스 번호로 공 1개를 추가로 추첨한다. 즉 1번부터 45번까지 중에서 6개를 추첨한 후 또 다시 1개를 추첨하는 것이다. 물론 이 번호들은 중복된 번호가 없다.

기능 구현 목록

Cypress 테스트

정상 로직
- 프로그램을 시작하면 구입금액 입력폼이 보인다.
- 사용자는 로또 구입 금액을 입력한다.
- 사용자가= 구매한 로또의 개수를 보여주고, 개수만큼 티켓 이모지를 가로로 보여준다. 지난주 당첨 번호 입력 폼을 보여준다.
- '번호 보기'를 눌렀을 때, 로또 아이콘을 세로로 정렬한다.
- '번호 보기'를 눌렀을 때, 각 로또의 번호를 보여준다. (로또의 번호는 중복이 없어야 한다)
예외상황
- 금액 입력 예외상황
  - 사용자가 금액을 1,000원 단위로 입력하지 않은 경우, alert로 알린다.
  - 구매 가능 금액은 최소 1,000원, 최대 100,000원으로 제한한다.

🚧 앱 구조

📺 디렉토리 구조

🤔 궁금한 점

현재 html 태그들의 class에 bootstrap classname이 들어가 있어서, 저희가 DOM을 가져오기 위해서 id를 사용했습니다. 평소에는 class에 태그의 이름을 지정하여 넣는 편이고 id는 잘 사용하지 않는데(이유는 딱히 생각하지 않고 썼던 것 같습니다.), 어떻게 쓰는 게 좋은 방식일까요? id는 unique하고 class는 unique하지 않다는 차이에서 실제로 더 일반적인 방식을 알고 싶습니다!
dom.js에서와 같이 선택자를 만들어주는 방식을 처음 적용해 보았는데, 괜찮은 방식인지 궁금합니다.

기타 질문은 코멘트로 달도록 하겠습니다!
리뷰 감사드립니다. 🙃

- update input price form's id

- create dom selector converter - add line spacing - update isValidPrice method for early return

zigsong · 2021-02-18T08:22:21Z

src/js/Lotto.js

+  constructor() {
+    this._numbers = new Set();
+  }


constructor에서 바로 new Set()과 같이 생성자 호출하는 것이 살짝 불안정해 보이는 느낌인데, 코드를 간결하게 하기 위해서 사용하였습니다. 괜찮은 방식일까요?

네 전혀 문제 없고 좋습니다.
[] 로 할당하는것보다 숫자가 겹치지 않는다는 의도를 전달 하기 위해 Set을 사용한 것도 좋고, constructor 에서 초기화하는 것도 적절한곳에 적절한 코드가 적절하게 위치하고 있습니다.

zigsong · 2021-02-18T08:25:12Z

src/js/LottoController.js

+    this.lottos = Array.from({ length: lottoCount }, () => {
+      const lotto = new Lotto();
+      lotto.initNumbers();
+      return lotto;
+    });
+  }


Array.from({ length: ... }), () = {}) 방식의 코드를 이번에 처음 접해 보았는데,
언젠가 new Array()와 같은 생성자 호출 방식은 좋은 방법이 아니며(😬 이 이유를 알고자 다시 검색해 보았으나 잘 나오지 않아서, 이 부분도 궁금합니다. ), 위 코드 역시 내부적으로 생성자를 호출하는 것이라서 좋은 코드가 아닐 수도 있다고 생각했습니다.

사용해도 좋은 방식인지 궁금합니다!

new 로 생성하면 객체의 생성 방법을 사용자측 코드에서 너무 자세히 알아야 하는 문제가 있습니다.(필수 파라미터가 무엇인지, 순서는 어떤지 등등 그래서 생성 방법이 바뀌면 해당 객체를 생성하는 모든 코드를 다 바꿔야 하는 문제가 있습니다.)
그래서 객체를 직접 생성하지 말고 요청하게 하는 것입니다.
객체를 생성하는 정적 메서드를 제공해주고 이 메서드를 사용하게 함으로써 앞서 있던 문제를 해결 할 수 있는것입니다.
'정적 팩토리 메서드 패턴'으로 검색하시면 자세한 내용을 접하실 수 있을것입니다.

정적 팩토리 메서드 패턴에 대해 간단하게 검색해 보았습니다. 처음 알게 되었습니다. 감사합니다 😊
번외로 제가 며칠 전에 Object.create()라는 문법을 알게 되었는데, 이것도 new Object가 아닌, .create()라는 메소드를 사용하여 새 객체를 생성하였으므로 정적 팩토리 메서드 패턴이라고 볼 수 있는지 궁금합니다! 😮

- remove unnecessary label 'for' attribute - create utils file for helper function - rename 'priceValidator' to 'validatePrice' - create getter for show lotto numbers - update logic for getting input price value - separate alert out of isValidPrice method

wow9144

안녕하세요. 역할 분리에도 신경을 쓰신거 같고 코드가 깔끔한편이어서 읽기 좋았습니다. :)

궁금한점 답변

class 명으로 셀렉팅하여 js에서 기능을 하게 하는것은 프로그램이 깨지기 쉬운, 굉장히 취약하게 만드는 방법중 하나 입니다. 기능의 변화없이 화면이 바뀌어 스타일을 새로 만들고 클래스를 바꿔주면 화면에 변화가 일어나고 기능은 똑같이 동작해야 합니다. (변한것은 화면의 디자인뿐이기 때문) 그러나 js에서 클래스에 의존하도록 구현 해놨었다면 이 기능은 망가지게 됩니다. 그래서 화면이 바뀌었는데 매번 js 에서 이걸 쓰는지 검색해가면서 조심스럽게 작업을 진행할 수 밖에 없어 시간도 더디게 됩니다. 따라서 단일 엘리먼트면 id 로 하도록 하고 여러개면(하나여도 상관없음) data-* 프로퍼티를 이용하면 좋습니다. data-section=lotto, data-section=result 이런식으로 하면 구성하는 섹션 전체를 가져올수도 그 안에서 세부적인 하나를 가져오기도 좋습니다.
현재의 $는 단순히 코드 타이핑을 줄이는 정도와 단일한 사용 포인트를 나의 코드레벨에서 관리 할 수 있다는 정도의 이점 밖에 없습니다.
따라서 $를 좀 더 개선하셔서 제이쿼리 처럼 show, hide 메서드를 제공하게 하면 좀 더 쓸만한 커스텀 라이브러리가 될 것입니다. 그리하면 LottoView의 show, hide는 제거 할 수도 있고 더 유연하게 쓸 수도 있습니다.
방법은 $ 함수가 엘리먼트를 바로 반환하지 말고 함수 내부에 지역변수로 두고, show,hide 메서드를 가진 객체를 반환하고 이 객체를 통해 element를 접근하게 하면 됩니다.
이것이 클로저를 이용한 모듈 패턴입니다.
$(셀렉터).show()
$(셀렉터).hide()
같은식으로 할 수 있게 됩니다.
필요에 따라 addEventListener, addClass, removeClass 같은것을 추가해보셔도 좋습니다.

wow9144 · 2021-02-21T07:38:52Z

src/js/Lotto.js

+
+export default class Lotto {
+  constructor() {
+    this._numbers = new Set();


private을 의도하셔서 _numbers 를 하신것이라면 좋습니다.
다만 이것은 자바스크립트에 private이 없을 때 관례적인 명명규칙일뿐 여전히 외부에서 직접 접근하여 조작 할 수 있습니다.
그러나 이제는 자바스크립트에도 공식적으로 private 의 의도를 표현 할 수 있는 #이 들어왔기 때문에
this.#numbers = ....
로 하시면 됩니다.

private을 의도한 것은 맞습니다!
다만 #을 쓰는 방식을 아직 지원하지 않는 브라우저들이 있다고 해서 사용을 고려했었고, 현재 safari 브라우저에서도 syntax 오류를 발생시켜 사용하지 않는 쪽으로 결정했습니다. 또한 크롬으로 실행하더라도 현재 eslint와 충돌이 나고, babel을 따로 설정해 줘야 해서 이번 단계에서는 사용하지 않기로 했습니다.

우선 현재는 numbers에 직접 접근할 일이 없어서, private 접근자(언더바)를 사용하지 않고 진행하기로 해보았습니다!

wow9144 · 2021-02-21T07:46:50Z

src/js/Lotto.js

+  }
+
+  initNumbers() {
+    while (this._numbers.size < NUMBERS.LOTT0_LENGTH) {


LOTT0_LENGTH 가 NUMBERS에 있는것보다 이 Lotto 에 스태틱 프로퍼티로 존재하는게 더 좋을것 같습니다.
Lotto는 몰라야하고 LOTT0_LENGTH 만 알아야 하는 객체가 있는 경우가 있는게 아니라면 LOTT0_LENGTH를 굳이 분리하는건 관리상 이점이 없을 것 같습니다.

그리고 LOTT0_LENGTH 보다는 LOTT0_MAX_LENGTH 가 더 의미적으로 명확할 것 같습니다.
근데 사용 할 땐 Lotto.LOTT0_LENGTH 가 될 테니 정보가 중복되므로 Lotto.MAX_NUMBER_LENGTH 정도의 의미 전달이 더 좋지 않을까 싶습니다.

각 로또별로 번호의 개수가 '최대' 6개가 아니라 '반드시' 6개여야 하기 때문에 MAX는 문맥에 맞지 않다고 생각했습니다!
말씀주신대로 해당 값은 Lotto 객체 안에 있는 게 좋을 것 같아서 그 부분은 수정해보도록 하겠습니다.

wow9144 · 2021-02-21T07:52:15Z

src/js/LottoController.js

+  createLottos(lottoCount) {
+    this.lottos = Array.from({ length: lottoCount }, () => {
+      const lotto = new Lotto();
+      lotto.initNumbers();


로또 객체를 생성하면 번호까지 완성된 로또가 튀어나오면 더 좋지 않을까 싶습니다.(자동)
외부에서 로또 객체를 만들고 번호까지 직접 init 해줘야 할 필요까지는 없을것으로 생각됩니다.
혹은 아예 생성할 때 번호를 직접 줘도 되고...(수동)
(이 부분은 지금 그냥 넘어가시고 다음 단계에서 고민하셔도 될 것 같습니다.)

리뷰 반영하도록 하겠습니다!
번호 수동 입력은 다음 단계 미션에 있기 때문에 그 때 참고하도록 하겠습니다 🤓

wow9144 · 2021-02-21T07:54:47Z

src/js/LottoController.js

+    const isSwitchOn = $lottoIconsDiv.checked;
+
+    if (isSwitchOn) {
+      $lottoIconsDiv.classList.add('flex-col');


컨트롤러에서 뷰의 세부적인 클래스(flex-col)까지 알고 있는것은 컨트롤러의 책임이 과합니다.
숨겨야 하고 보여야 할 게 있다면 단지 뷰에 요청만 하면 되고 flex-col 같은 것은 뷰에서 알아야 할 정보라고 보여집니다.

현재 View와 Controller의 역할을 다시 고민해 보고 각각의 역할에 맞게 분리하는 작업을 하고 있어서, 2단계 미션에 전반적인 리팩토링을 추가할 예정입니다. 말씀 주신 부분도 해당 리팩토링 단계에서 참고하여 포함하도록 하겠습니다. 좋은 조언 감사합니다 😊

wow9144 · 2021-02-21T08:03:25Z

src/js/utils/constants.js

@@ -0,0 +1,13 @@
+export const NUMBERS = {


NUMBERS 라는 이름의 constants를 만들게 되면 실제로 그 내부의 있는것들은 전혀 연관관계가 없지만 숫자라는 이유만으로 같이 뭉쳐있게 됩니다. (이내 쓰레기통과 같은 객체가 됩니다.)
예를 들면 키, 몸무게, 나이, 생년월일, 핸드폰번호 같은것은 개인정보라는 카테고리로 묶여있어야 하고 100미터 달리기 기록, 2분동안 푸시업 개수, 풀업 개수 같은것은 운동 관련 카테고리로 묶여 있어야 합니다.
따라서 NUMBERS라는 상수 객체가 생겨버리면
const NUMBERS = {
키: xx,
몸무게: xx,
푸시업개수: xx
로또최대숫자: xx,
}
처럼 되길 의도한 이름이라고 봐도 무리가 없기 때문에(물론 그런 의도는 아니었겠지만...) 그래서 이름을 좀 더 구체적으로 잘 지어주셔야 겠습니다.

나중에 바꾸려고 했는데 실수로 바꾸지 않은 채로 올렸네요 😅 좋은 지적 감사합니다! 반영하여 수정하였습니다.

zigsong · 2021-02-21T15:21:52Z

dataset을 활용한 방식도 권장되는군요! 사용해 보도록 하겠습니다
View에서 공통적으로 사용되는 메소드들을 모아서 부모 View class를 만들고, 해당 class를 상속하는 방식으로 코드를 작성해보려고 합니다. 현재 Controller와 View의 역할을 분리하여 DOM 조작과 관련된 코드를 View에서만 작성하도록 할 계획입니다. 리팩토링을 하면서 참고하여 반영해보도록 하겠습니다. 감사합니다 ☺️

- change unused private property to public property - redefine Lotto Length property to Lotto class static property - move initNumbers method to constructor - rename constant name in specific way - install eslint-babel to fix eslint syntax

zigsong · 2021-04-24T08:59:44Z

학습 로그

정적 팩토리 메서드 패턴 ref

new 키워드로 생성하면, 객체의 생성 방법을 사용자측에서 너무 자세히 알아야 하는 문제가 있기 때문이다. 객체를 생성하는 정적 메서드를 제공해주고 이 메서드를 사용하게끔 하여 문제를 해결할 수 있다.

객체 생성을 캡슐화하는 기법

객체 생성의 역할을 하는 클래스 메서드 (static)

생성자 대신 정적 팩토리 메서드를 사용할 수 없는지 생각해보기

class Lotto = {
 constructor() {
  // ... 
 }

 static newLotto() {
  return new Lotto(genajerateRandomNumbers());
 }
}

// bad
const lotto = new Lotto(generateRandomNumbers());

// good
const lotto = Lotto.newLotto();

링크
- https://johngrib.github.io/wiki/static-factory-method-pattern/
- https://velog.io/@ljinsk3/정적-팩토리-메서드는-왜-사용할까

DOM 선택자로 class명 대신 id(단일 엘리먼트), dataset(여러 개) 사용하기 ref

class명으로 선택하는 경우 기능과 상관없이 스타일이 바뀌었을 때 일일이 찾아 수정해주어야 하는 번거로움이 있으며, 프로그램이 깨지기 쉬운 취약한 구조이다.
문서에 한 개뿐인 단일 요소이면 id를, 특정한 데이터 속성을 가지고 있다면 dataset을 사용하자
dataset은 id와 class가 적절하지 않은 상황에서만 사용하자
링크
- https://stackoverflow.com/questions/17184918/best-practice-class-or-data-attribute-as-identifier/17185248

class private field, hash `#` (ref)

ES2019에서 추가된 문법

# prefix를 추가해 private class field를 선언할 수 있다

class ClassWithPrivateField {
  #privateField
}

class ClassWithPrivateMethod {
  #privateMethod() {
    return 'hello world'
  }
}

class ClassWithPrivateStaticField {
  static #PRIVATE_STATIC_FIELD
}

private class field는 class 선언문 내부의 class 생성자에서 접근이 가능하다
링크
- https://developer.mozilla.org/ko/docs/Web/JavaScript/Reference/Classes/Private_class_fields

devhyun637 and others added 29 commits February 16, 2021 16:45

chore : project settings & config linter

bc8891f

docs : update TodoList

a672951

test : show price form only when started

53a2a72

feat : show price form only when started

93ba66d

docs : update README.md

5772391

test : show lotto views when typed price and click confirm

a7d969b

- update input price form's id

feat : show lotto views when typed price and click confirm

f64652e

chore : apply eslint syntax

2568792

test : show total lotto counts and lotto icons

5f5ea9e

feat : show total lotto counts and lotto icons

d76a6f0

test : align lotto icons vertically if clicked switch

5dd7839

docs : update README.md

eee6de7

feat : switch lotto icons alignment if clicked toggle

3d1bd75

test(refactor) : switch lotto icons alignment

ae6e557

test : show lotto numbers when switch on

3ab7781

feat : show lotto numbers when switch on

dfe0d5c

chore : update eslint

1fd7fd6

test(refactor) : update element classname and visibility logic

692d916

style : adjust lotto detail font size

a5c7949

test : check price input unit validation

e5c68ed

feat : check price input unit validation

a508dc4

test : check price input in range validation

cef5f40

feat : check price input in range validation

23c386a

feat : enable input price form autofocus

751924f

test : add enter event on input price form

b600c82

feat : update sumbit event on input price form

15ec627

test(refactor) : group duplicated code into a function

19fe96f

refactor : separate methods in controller

789d82e

refactor : refactor code

20a872c

- create dom selector converter - add line spacing - update isValidPrice method for early return

zigsong self-assigned this Feb 18, 2021

docs : update README.md

62fc499

pocojang requested a review from wow9144 February 18, 2021 08:18

zigsong commented Feb 18, 2021

View reviewed changes

pocojang assigned wow9144 Feb 18, 2021

devhyun637 and others added 8 commits February 18, 2021 19:28

docs : update README.md

8005e34

docs : update README.md

7a2a892

refactor : apply code reviews

19f6aef

- remove unnecessary label 'for' attribute - create utils file for helper function - rename 'priceValidator' to 'validatePrice' - create getter for show lotto numbers - update logic for getting input price value - separate alert out of isValidPrice method

fix : resolve conflict from previous pair refactoring

3b445bf

refactor : refactor markup to be semantic

bfde4c3

test(refactor) : update input price validation logic

98a7bdd

refactor : update input price validation logic

1b655b3

refactor : update lotto detail div to list

d5a3ac1

wow9144 reviewed Feb 21, 2021

View reviewed changes

refactor :

2b228d3

- change unused private property to public property - redefine Lotto Length property to Lotto class static property - move initNumbers method to constructor - rename constant name in specific way - install eslint-babel to fix eslint syntax

zigsong mentioned this pull request Feb 25, 2021

[2단계 - 행운의 로또 미션] 지그(송지은) 미션 제출합니다. #43

Merged

23 tasks

zigsong requested a review from wow9144 February 27, 2021 03:27

wow9144 merged commit 14f63a1 into woowacourse:zigsong Feb 27, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1단계 - 행운의 로또 미션] 지그(송지은) 미션 제출합니다. #23

[1단계 - 행운의 로또 미션] 지그(송지은) 미션 제출합니다. #23

zigsong commented Feb 18, 2021 •

edited by seojihwan

Loading

zigsong Feb 18, 2021

wow9144 Feb 21, 2021

zigsong Feb 18, 2021

wow9144 Feb 21, 2021

zigsong Feb 21, 2021 •

edited

Loading

wow9144 left a comment

wow9144 Feb 21, 2021

zigsong Feb 21, 2021 •

edited

Loading

wow9144 Feb 21, 2021

zigsong Feb 21, 2021

wow9144 Feb 21, 2021

zigsong Feb 21, 2021

wow9144 Feb 21, 2021

zigsong Feb 21, 2021

wow9144 Feb 21, 2021

zigsong Feb 21, 2021 •

edited

Loading

zigsong commented Feb 21, 2021

zigsong commented Apr 24, 2021

[1단계 - 행운의 로또 미션] 지그(송지은) 미션 제출합니다. #23

[1단계 - 행운의 로또 미션] 지그(송지은) 미션 제출합니다. #23

Conversation

zigsong commented Feb 18, 2021 • edited by seojihwan Loading

✅ 1단계 미션 TODO

🚧 앱 구조

📺 디렉토리 구조

🤔 궁금한 점

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zigsong Feb 21, 2021 • edited Loading

Choose a reason for hiding this comment

wow9144 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zigsong Feb 21, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zigsong Feb 21, 2021 • edited Loading

Choose a reason for hiding this comment

zigsong commented Feb 21, 2021

zigsong commented Apr 24, 2021

학습 로그

정적 팩토리 메서드 패턴 ref

DOM 선택자로 class명 대신 id(단일 엘리먼트), dataset(여러 개) 사용하기 ref

class private field, hash # (ref)

zigsong commented Feb 18, 2021 •

edited by seojihwan

Loading

zigsong Feb 21, 2021 •

edited

Loading

zigsong Feb 21, 2021 •

edited

Loading

zigsong Feb 21, 2021 •

edited

Loading

class private field, hash `#` (ref)