[1단계 - 자판기] 도리(박선호) 미션 제출합니다. #28

prefer2 · 2022-03-27T07:06:35Z

안녕하세요 리뷰어님. 🐠도리🐠라고 합니다. 이번 자판기 미션동안 잘부탁드려요!

데모 페이지

라우터의 경우, 미션에서 주어진대로 Browser History Api를 사용해보고 싶었으나, 서버없이 새로고침을 동작하게 하는 방법을 찾지 못해 hash방식을 사용하였습니다. hash값이 변할때마다 window.location으로 이를 찾아 원하는 페이지가 나오도록했습니다.

도메인 영역에만 typescript를 사용해보았습니다. 처음으로 typescript를 사용해서 부족한 부분이 많은 것 같습니다. 보시면서 수정하면 좋을 부분, 더 학습하면 좋을 부분들 추천해주시면 감사하겠습니다!

기능 구현

추가된 상품이 5개가 넘을 경우 table body에 스크롤이 가능합니다
동일한 이름의 상품을 추가한 경우, 가격은 나중에 추가한 값으로 변경되고 수량은 합산됩니다
이미 존재하고 있는 이름으로는 수정이 불가능합니다

아쉬운점

여러개의 아이템 수정 시 한개의 아이템만 수정을 완료 해도 모든 아이템들이 재랜더링 됩니다. 확인 버튼을 누른 값만 수정되고 다른 값들은 수정할 수 있도록 남아있으면 좋겠지만 수정을 시도한 모든 값들이 재랜더링됩니다. items state가 변화되서 이와 관련된 모든 값들이 랜더링되는 결과입니다. 이에 관한 리뷰어님의 의견이 궁금합니다!

Co-authored-by: SunHo Park <sunho6200@gmail.com>

- npm: dependecies 설치, eslint, prettier, jest 설치, lint 스크립트 추가 - eslint 설정 - prettier: prettier-recommended 설정, single quote 강제 - .gitignore: .DS_Store, .vscode 추가 Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

상품 추가 시 예외사항 추가 Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

path가 '*'이면 default route component로 route Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

lsw1164

안녕하세요 도리님 코드 잘봤습니다.

코멘트 남겼으니 참고 부탁드립니다!

lsw1164 · 2022-03-27T13:17:14Z

src/core/Component.js

+    while (this.firstChild) {
+      this.removeChild(this.lastChild);
+    }


replaceChildren을 활용하면 while문이 필요 없겠군요.

https://developer.mozilla.org/en-US/docs/Web/API/Element/replaceChildren

몰랐던 방법인데 감사합니다! Component와 TableRow 모두 수정했습니다

lsw1164 · 2022-03-27T13:18:56Z

src/core/Component.js

+  }
+
+  template() {
+    throw new Error('override');


override 강제하는 코드 좋네요 👍

lsw1164 · 2022-03-27T13:20:04Z

src/core/Component.js

+  afterRender() {}
+
+  setState(newState) {
+    if (deepEqual(this.state, newState)) return;


상태 변화가 없을 때는 렌더링하지 않는군요 👍

lsw1164 · 2022-03-27T13:33:14Z

src/core/Subject.ts

+    return observableObj;
+  }
+
+  static observe(target: Observer) {


static 메서드로 observe가 있고 일반 메서드 observe가 또 있는건가요?
어떤 의도인지 파악하기 어렵습니다 ㅠ

static 메서드는 옵저버가 렌더링 시 참조하는 subject들을 자동으로 랜더하고 구독하기 위해 사용했습니다.
컴포넌트의 render를 실행시켜서 컴포넌트가 렌더링시 참조하는 subject.value의 setter 호출합니다. setter가 호출됨으로서 currentObserver에 담긴 컴포넌트를 자신의 observer로 추가할 수 있습니다.

connectedCallback() { Subject.observe(this); } disconnectedCallback() { Subject.unobserve(this); }

connectedCallback으로 요소가 DOM에 추가될 때마다 렌더링을 하게 됩니다!

일반 메서드 observe는 observer를 등록합니다.

이름이 동일해서 많이 헷갈리네요. static은 초기값을 관찰, 일반 observe는 observer로 등록을 의미했습니다. 조금 더 명확하게 하기 위해 일반 observe를 addObserver로 변경했습니다! 32ec560

lsw1164 · 2022-03-27T13:38:58Z

src/core/Subject.ts

+  get(): any {
+    if (Subject.currentObserver) this.observe(Subject.currentObserver);
+
+    return this.value;
+  }


get이 무슨 값을 얻어오는지 사용자 입장에서 알기 어렵습니다.

get을 하는데 observe를 호출하는 의도를 알 수 있을까요?

Object.defineProperty(observableObj, key, { get() { return subject.get(); }, set(newValue) { subject.set(newValue); },

위 코드에서 값을 받아오기 위해 사용합니다. defineProperty로 접근자를 추가하기 위해서 사용했습니다. 객체.key 형식으로 접근(get)하게 되면 this.value(객체의 state)가 리턴되게 하였습니다.

get을 하는 값들은 state와 연관이 있기 때문에 observe해주었습니다. set을 통한 state 변화가 있을 때, 등록된 observer들에게 알릴 수 있도록 observe(구독)해주었습니다.

lsw1164 · 2022-03-27T13:45:29Z

src/core/Subject.ts

+  notify(): void;
+}
+
+export default class Subject {


Subject의 역할을 무엇이라고 이해하면 좋을까요?

앞으로 observer들이 구독할 주제(이벤트를 발생시키는 주체)라고 생각하면 될 것 같습니다. 이번 미션에서는 VendingMachine이라는 주제 밖에 없어서 사용이 많지는 않을 것 같네요🙃 Subject는 set,get마다 상태 변화를 감지하고 render와 같은 일들을 해줍니다. 이를 직접 VendingMachine의 상태 변화가 있을 때마다 직접 해줄 수 있지만(ex. addItem시 items state가 변하므로 내부에서 observer들을 돌며 render를 호출) 이를 따로 해주지 않기 위해 만들었습니다!

Subject 클래스를 작성하면서 너무 복잡한가...? 라는 생각을 했던 것 같네요. 리트님이 보기에는 어땠는지 궁금하네요 🧐

Subject, Observer 객체를 활용해서 옵저버 패턴 구현을 의도했구나 생각했습니다.

Subject 안에 있는 인터페이스들이 혼동의 여지가 있어서 복잡하다고 느꼈습니다.

Subject에서 static 메서드를 사용한 의도를 파악하기 어려웠습니다.
(서로 다른 성격의 Subject들이 모두 한 곳에서 관리하는 걸 의도한 걸까요? Subject의 역할이 구독한 옵저버들에게 이벤트를 발생시키는 역할이라고 해주셨는데요. 서로 다른 Subject들을 한 곳에서 관리해주는 건 Subject의 역할을 벗어난 다른 책임이라는 생각이 들었습니다.)

lsw1164 · 2022-03-27T13:50:07Z

src/utils/validator.ts

+  errorMessage: string;
+}
+
+type Validator = Array<Condition>;


validate 포맷을 공통화했군요 👍

lsw1164 · 2022-03-27T13:54:33Z

src/views/App.js

+        </header>
+        <nav-bar class="nav-bar"></nav-bar>
+        <div class="page-container">
+          <page-router>


Router 사용성이 좋아 보입니다 👍

lsw1164 · 2022-03-27T14:03:31Z

src/views/pages/NotFoundPage.js

@@ -0,0 +1,13 @@
+import Component from '../../core/Component';
+
+class NotFound extends Component {


아직 쓰이지 않는 컴포넌트인가요?

올바르지 않은 url 접근시 404 Error를 보여주기 위한 컴포넌트입니다. App에서 정해진 url이 아닌 값들을 접근할 시에 띄우고 있습니다. 사용자가 알아보기 좋도록 문구 추가했습니다! f34160e

혹시 에러 페이지에 관해 더 추가해 볼 내용이 있을까요?? 제가 보기에도 조금 빈약해보이네요 😅

앗 사용하는 곳 확인했습니다 :)

lsw1164 · 2022-03-27T14:04:31Z

src/core/TableRow.js

+import Subject from './Subject';
+import { deepEqual } from '../utils/commons';
+
+export default class TableRow extends HTMLTableRowElement {


TableRow는 Component랑 어떤 차이가 있다고 보면 될까요?

ItemRow를 위해 만들었습니다. tr 속성을 가진 customElement를 생성하고 싶었는데 HTMLElement를 상속받아서는 이를 구현할 수 없었습니다. customElement의 인자로 { extends: 'tr' }을 줘봤는데도 전혀 반영이 되지 않았습니다.

table의 속성을 가진 Element를 만드는 방법을 찾다 HTMLTableRowElement를 발견하고 이를 적용시켰습니다. 동작은 동일하고 Component와의 차이는 table의 속성을 가지고 있는가에만 있습니다. HTMLTableRowElement

혹시 HTMLElement만으로도 table 속성을 가지게 하는 방법이 있을까요....?

두 클래스가 중복된 메서드들이 많은 것 같아 최대한 중복을 제거해보았습니다. 다중 상속에 대해서 찾다가 찾은 방법인데 이런 식으로 해도 괜찮은가요??? 11107a5

혹시 HTMLElement만으로도 table 속성을 가지게 하는 방법이 있을까요....?

제가 아는 선에선 없습니다.

두 클래스가 중복된 메서드들이 많은 것 같아 최대한 중복을 제거해보았습니다. 다중 상속에 대해서 찾다가 찾은 방법인데 이런 식으로 해도 괜찮은가요???

네, 자바스크립트는 기본적으로 다중 상속을 지원하지 않기 때문에 찾으신 방법으로 중복을 해결할 수 있겠네요.
하지만, 어떤 방법을 찾으실 때 그 방법의 주의할 점 & 단점 & 사이드 이펙트를 인지하는 건 필수입니다.
직접 찾아보는 과정에서 얻어가실 게 더 많을거라 생각합니다.
(prototype, super 키워드에 대한 이해가 필요할 거라 생각됩니다)

lsw1164 · 2022-03-27T14:19:50Z

여러개의 아이템 수정 시 한개의 아이템만 수정을 완료 해도 모든 아이템들이 재랜더링 됩니다. 확인 버튼을 누른 값만 수정되고 다른 값들은 수정할 수 있도록 남아있으면 좋겠지만 수정을 시도한 모든 값들이 재랜더링됩니다. items state가 변화되서 이와 관련된 모든 값들이 랜더링되는 결과입니다. 이에 관한 리뷰어님의 의견이 궁금합니다!

이걸 개선하려면 현재 dom에서 바뀌려는 부분만 체크해서 렌더링하는 로직을 구현해야됩니다.
다른 오픈소스 프레임워크 & 라이브러리에서도 바뀔 부분만 구분해서 그리는 로직을 내부적으로 수행하고 있습니다.
잘 구현하시면 좋겠지만 diff 알고리즘 구현이 현 미션의 핵심은 아니라고 생각합니다.
궁금하시다면 별도로 리서치해보시면 좋겠습니다 :)

prefer2 · 2022-03-29T14:59:50Z

이걸 개선하려면 현재 dom에서 바뀌려는 부분만 체크해서 렌더링하는 로직을 구현해야됩니다.
다른 오픈소스 프레임워크 & 라이브러리에서도 바뀔 부분만 구분해서 그리는 로직을 내부적으로 수행하고 있습니다.
잘 구현하시면 좋겠지만 diff 알고리즘 구현이 현 미션의 핵심은 아니라고 생각합니다.
궁금하시다면 별도로 리서치해보시면 좋겠습니다 :)

이 부분은 아직 너무 어렵네요. 이 미션의 핵심이 아닌 만큼 어떤 식으로 작동하는지 개념 정도만 살펴보았습니다.

주신 리뷰들 반영해보았습니다! 질문에 대한 답변, 답변하면서 궁금했던 점들 코멘트로 남겨보았습니다. 코멘트 참고 부탁드립니다!

lsw1164

도리님, 피드백 반영 확인했습니다.
이번 스텝은 Approve 하고 나머지 피드백은 다음 스텝 진행하시면서 반영해주시면 되겠습니다!

lsw1164 · 2022-03-30T13:14:40Z

src/core/Subject.ts

+  notify(): void;
+}
+
+export default class Subject {


Subject, Observer 객체를 활용해서 옵저버 패턴 구현을 의도했구나 생각했습니다.

Subject 안에 있는 인터페이스들이 혼동의 여지가 있어서 복잡하다고 느꼈습니다.

Subject에서 static 메서드를 사용한 의도를 파악하기 어려웠습니다.
(서로 다른 성격의 Subject들이 모두 한 곳에서 관리하는 걸 의도한 걸까요? Subject의 역할이 구독한 옵저버들에게 이벤트를 발생시키는 역할이라고 해주셨는데요. 서로 다른 Subject들을 한 곳에서 관리해주는 건 Subject의 역할을 벗어난 다른 책임이라는 생각이 들었습니다.)

lsw1164 · 2022-03-30T13:49:45Z

src/core/TableRow.js

+import Subject from './Subject';
+import { deepEqual } from '../utils/commons';
+
+export default class TableRow extends HTMLTableRowElement {


혹시 HTMLElement만으로도 table 속성을 가지게 하는 방법이 있을까요....?

제가 아는 선에선 없습니다.

두 클래스가 중복된 메서드들이 많은 것 같아 최대한 중복을 제거해보았습니다. 다중 상속에 대해서 찾다가 찾은 방법인데 이런 식으로 해도 괜찮은가요???

네, 자바스크립트는 기본적으로 다중 상속을 지원하지 않기 때문에 찾으신 방법으로 중복을 해결할 수 있겠네요.
하지만, 어떤 방법을 찾으실 때 그 방법의 주의할 점 & 단점 & 사이드 이펙트를 인지하는 건 필수입니다.
직접 찾아보는 과정에서 얻어가실 게 더 많을거라 생각합니다.
(prototype, super 키워드에 대한 이해가 필요할 거라 생각됩니다)

greenblues1190 and others added 30 commits March 22, 2022 15:05

docs: 1단계 요구사항 작성

d37910a

Co-authored-by: SunHo Park <sunho6200@gmail.com>

docs: 요구사항 수정

b544be3

상품 추가 시 예외사항 추가 Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

chore: jest 환경설정

fb823c5

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

test: ItemService.add 테스트 코드 작성

65b38d8

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

feat: ItemService.add 구현

9032c28

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

test: itemService.update 테스트 코드 작성

89c6d0b

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

feat: ItemService.update 구현

c5bf140

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

test: ItemService.delete 테스트 코드 작성

b6e6685

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

feat: ItemService.delete 구현

4d6bc32

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

test: ItemService.find 테스트 코드 작성

d2400c1

feat: ItemService.find 구현

149f25c

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

test: CoinService.add 테스트 코드 작성

43d4eb9

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

feat: CoinService.add 구현

dbf5e0f

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

test: CoinService.getTotalMoney 테스트 코드 작성

b9a24d9

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

refactor: 폴더 구조 수정

4feda40

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

feat: Component 구현

965241e

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

feat: page 구현

c6d57b5

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

feat: 동작만 하는 라우터 구현

f080bbe

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

feat: 라우팅 hash 방식으로 수정

a3775b0

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

refactor: 라우터 리팩터링

0f76ca1

path가 '*'이면 default route component로 route Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

test: 도메인 변경에 따른 테스트 코드 수정

f0f4e1d

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

refactor: 도메인 수정

db32ab8

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

fix: 초기 페이지 로드 시 주소로 라우팅

570f131

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

feat: UI 구현, 도메인 기능 바인딩

d680df1

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

refactor: items를 object로 수정, Subject 구현

6ee5f83

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

refactor: Component에 Subject 구독, 구독해제 추가

724fc17

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

refactor: validator 분리

fc027e6

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

refactor: 파일명, addCoin, 에러핸들링 수정

444787e

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

refactor: 도메인 타입 수정, ts 환경설정

6f8d20e

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

greenblues1190 and others added 12 commits March 26, 2022 18:24

style: App, ItemmanagementPage css 설정

ef394af

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

refactor: 상품 수정 시 포커싱, 잔동충전탭 css 추가

c5539b6

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

chore: eslint 규칙 추가

fb273c1

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

chore: dist 커밋

0f6366f

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

chore: gh-pages 설정

c781eac

Co-authored-by: SunHo Park <prefer2@users.noreply.github.com>

chore: 기본 설정

e3b6f45

refactor: css 주석 제거

5bfd024

refactor: 초기 tab 선택 설정

5a7c37e

chore: deploy 재설정

49110c5

refactor: 상수 분리 및 파일 정리

9130cc0

refactor: css 변수 분리 및 수정

bc3ed49

refactor: 상수 분리 추가 및 재배포

4e5f767

pocojang requested a review from lsw1164 March 27, 2022 07:07

pocojang assigned lsw1164 Mar 27, 2022

prefer2 changed the base branch from main to prefer2 March 27, 2022 07:14

lsw1164 requested changes Mar 27, 2022

View reviewed changes

prefer2 added 2 commits March 28, 2022 16:35

refactor: removeChild -> removeChildren 변경

d5d736b

refactor: observe, unobserve 메서드 이름 변경

32ec560

prefer2 force-pushed the step1 branch from e21d711 to 32ec560 Compare March 29, 2022 13:01

prefer2 added 7 commits March 29, 2022 22:01

refactor: checkUpdate -> detectUpdate 메서드명 변경경

125aeff

refactor: 불필요한 spread, 문 삭제 및 매개변수 명 변경경

528e275

refactor: filter -> find 변경

420a922

refactor: component, tableRow 중복 제거

11107a5

refactor: 404 error 문구 추가

f34160e

fix: 동일한 아이템 추가 시 최근 가격으로 업데이트 되도록 수정

5809f86

refactor: 가독성을 위해 early return

27dc812

lsw1164 approved these changes Mar 30, 2022

View reviewed changes

lsw1164 merged commit 6028577 into woowacourse:prefer2 Mar 30, 2022

		@@ -0,0 +1,13 @@
		import Component from '../../core/Component';

		class NotFound extends Component {

[1단계 - 자판기] 도리(박선호) 미션 제출합니다. #28

[1단계 - 자판기] 도리(박선호) 미션 제출합니다. #28

Uh oh!

Conversation

prefer2 commented Mar 27, 2022

기능 구현

아쉬운점

Uh oh!

lsw1164 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!

prefer2 Mar 29, 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!

prefer2 Mar 29, 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!

prefer2 Mar 29, 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!

prefer2 Mar 29, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

prefer2 Mar 29, 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!

prefer2 Mar 28, 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!

prefer2 Mar 29, 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!

lsw1164 Mar 30, 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!

lsw1164 commented Mar 27, 2022

Uh oh!

prefer2 commented Mar 29, 2022

Uh oh!

lsw1164 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

prefer2 Mar 29, 2022 •

edited

Loading

prefer2 Mar 29, 2022 •

edited

Loading

prefer2 Mar 29, 2022 •

edited

Loading

prefer2 Mar 29, 2022 •

edited

Loading

prefer2 Mar 29, 2022 •

edited

Loading

prefer2 Mar 28, 2022 •

edited

Loading

prefer2 Mar 29, 2022 •

edited

Loading

lsw1164 Mar 30, 2022 •

edited

Loading

lsw1164 Mar 30, 2022 •

edited

Loading