[2단계 - 자판기] 호프(김문희) 미션 제출합니다. #61

moonheekim0118 · 2022-04-07T06:09:20Z

안녕하세요 헤인티! 오랜만에 뵙습니다 !
헤인티가 주신 피드백 반영하면서 정말 많이 학습할 수 있었어서 좋았어요!

로컬에서 실행하는 법

npm run server
npm run start

먼저 저번에 주신 enum, as const, readonly 에 대해 학습해봤는데요, 아래와 같이 정리 할 수 있었습니다. 아래 정리하면서 타입스크립트에 대해서 많이 학습 할 수 있었어요! 감사드립니다.

enum 을 사용하면 안되는 이유

결과적으로 말하면 enum을 사용하면 자바스크립트에서 Tree Shaking 이 되지 않습니다.
- Tree Shaking 이란 사용하지 않는 코드를 삭제하는 기능
- Tree Shaking 을 통해 export 했지만, 아무데서도 import 하지 않는 모듈을이나 사용하지 않는 코드를 삭제하여 번들 크기를 줄여 페이지가 렌더링 되는 시간을 단축 할 수 있습니다.
enum 은 JavaScript에 존재하지 않고, TypeScript 가 구현한 코드이기 때문에, 즉시실행 함수를 통해 구현됩니다.
그런데 번들러는 즉시실행함수를 ‘사용하지 않는 함수’라고 판단 할 수 없기 때문에 Tree-Shaking 이 적용되지 않습니다.

readonly 타입이란?

readonly type 은 인터페이스 속성을 readonly (읽기전용) 으로 지정해줍니다.

type Foo {
	readonly bar: number;
	readonly bas: number; 
}

let foo:Foo = { bar:'bar', bas,:'bas' }; // ok

foo.bar = 'bas'; // Error

위 처럼 readonly 타입으로 지정된 객체 속성은, 초기화는 가능하지만 변경이 불가능 합니다.

as const 란?

as const 를 사용해서 , 타입명으로 타입핑을 하는 대신, ‘const’ assertion을 적용합니다.
as const 가 적용된 변수는 아래와 같은 특성을 가집니다.

타입이 확장되지 않습니다.
- 예를 들어, const foo = 'bar' as const 라고 선언 할 경우, 변수 foo의 타입은 string으로 확장되지 않고, bar가 됩니다.
객체 리터럴에 as const를 지정할 경우, 객체 프로퍼티에 readonly 속성이 지정됩니다.

좋은 코드리뷰 너무 감사드리구, 부족한 코드를...리뷰요청드리게 된 점 정말 죄송합니다. 😭

wmakerjun

호프 어려운 자판기 2단계 미션 진행하느라 너무너무 고생 많이했어요~ 😆

피드백 드릴 부분은 코멘트에 남겼는데요 아래 2가지 내용은 자주 코멘트로 언급한것 같아서 한번 더 정리했어요~

특히 네이밍은 저도 어려운 부분이고 늘 고민이 많이 드는데요 ㅎㅎ 지금부터 고민해보고 같은 기능을 다른 사람들은 어떻게 보게 될까를 생각해보면 조금씩 더 잘하게 될 수 있는것 같아요. 참고로 미래의 나도 다른 사람이라고 생각하면 더 의미있는것 같아요 😉

네이밍

메서드 네이밍을 보고 어떤 동작 또는 반환값이 있을지 상상해보면서 봤을 때 추측하기 어려운 부분들이 있었던것 같아요. 먼저 동작 가능한 코드를 작성하면서 충분히 고민할 시간이 부족했던 부분도 있을텐데요. 리팩터링 하면서 한번더 네이밍을 보고 어떤 동작 또는 반환값을 주는지 드러낼 수 있게 만들어 보면 어떨까요~?

메세지 상수화

confirm, snackbar의 경우 상수화한 메시지를 활용하는데, alert같은 경우에는 그렇지 않은 경우들이 있는것 같아요. 통일해보면 어떨까요~?

wmakerjun · 2022-04-08T07:52:37Z

src/css/index.css

+
+.snackbar.error{
+    background-color: var( --snackbar-error-bg-color);
+}


EOL이 등장했어요!

wmakerjun · 2022-04-08T07:52:47Z

src/css/loginPage.css

+
+
+


앗 이 공백은...!?

wmakerjun · 2022-04-08T07:53:45Z

src/js/api/index.ts

+  signUp: async (option) => {
+    try {
+      const response = await request("/register", {
+        method: "POST",
+        headers: baseHeader,
+        body: JSON.stringify(option),
+      });
+      return { ...response, isError: false };
+    } catch (error) {
+      return { isError: true };
+    }
+  },
+
+  login: async (option) => {
+    try {
+      const response = await request("/login", {
+        method: "POST",
+        headers: baseHeader,
+        body: JSON.stringify(option),
+      });
+      return { ...response, isError: false };
+    } catch (error) {
+      return { isError: true };
+    }
+  },


구조가 비슷한 메서드를 한단계 추상화해서 만들어보면 어떨까요~? 재사용가능한 부분들이 보이는것 같아요 🤩

지금보니 반복되는 부분이 많아서 준 리뷰 대로 조금 수정을 해봤습니다. 특히나 try-catch 에서 response 값을 객체 형태로 넘겨주는 부분은 모두 다 동일하기 때문에, 아예 request 함수에서 처리해주도록 했습니다.

물론 '회원가입' '로그인' '유저정보 수정' 등..의 여러 api 요청 메서드들을 한단계 추상화해서, 외부에서 매개변수만 변경하여 재사용 할만한 메서드 하나를 만들 수 있을 것 같긴하지만 그럴 경우 도메인로직에서 해당 메서드를 사용할 때, 헤더 정보나 url 정보를 불필요하게 알아야한다고 생각해서 말씀해주신 추상화는 하지 않았어요! 이 부분에 대해서 어떻게 생각하시나요~~?

bc1144d

한단계 추상화했을 뿐만 아니라, 객체의 책임과 관련된 부분까지 고민해서 작업한 점이 너무 인상적이에요 👍🏻
그런데 const data = await response.json();이 부분에서 response.json()이 실패한 경우에 에러 처리는 어떻게 해줘야할지도 고민되는 것 같아요. fetch API 사용하는 로직은 앞으로도 계속 사용하는 부분이니 이 미션이 아니어도, 다음 미션에서 반영해보면 좋을 것 같아요!

wmakerjun · 2022-04-10T03:31:39Z

src/css/index.css

+
+

사소하지만 이런 공백라인도 동일하게 맞춰가면 좋을 것 같아요 😄

오..사소한 부분까지 알려주셔서 감사합니다 😄
61fa29c

wmakerjun · 2022-04-10T03:35:55Z

src/js/app.ts

+    this.router = {
+      [HASH.PRODUCT_MANAGEMENT]: () => {
+        this.productModerator.init();
+      },
+      [HASH.CHARGE_CHANGES]: () => {
+        this.changesModerator.init();
+      },
+      [HASH.PRODUCT_PURCHASE]: () => {
+        this.purchaseModerator.init();
+      },
+      [HASH.SIGNUP]: () => {
+        this.signUpModerator.init();
+      },
+      [HASH.LOGIN]: () => {
+        this.loginModerator.init();
+      },
+      [HASH.USER_INFO]: () => {
+        this.userInfoModerator.init();
+      },
+    };


준에게 배운것.... 👍 👍

wmakerjun · 2022-04-11T02:18:17Z

src/js/moderator/productModerator.ts

    try {
-      const product = this.productProcessMachine.update(id, name, price, count);
+      const product = this.vendingMachine.updateProduct(id, name, price, count);


addProduct의 경우 객체로 인자를 넣고, updateProduct는 4개의 인자를 순서대로 넣는데 혹시 일관되게 구현하지 않는 이유가 있을까요~?

급하게 하느라 통일성을 지키지 못했네요 😭 😭 이 부분 수정해서 둘다 객체로 인자를 받도록 했습니다. 이렇게 사소한 부분까지 짚어주셔서 감사해요!
9716133

wmakerjun · 2022-04-11T02:20:38Z

src/js/ui/productPageView.ts

    $("#products-list", this.$productStatusContainer).insertAdjacentHTML(
      "beforeend",
      productTemplate.product(product.get())
    );
  }

-  renderDeleteProduct(id: string): void {
+  renderDeleteProduct(id: string) {


삭제된 프로덕트를 render한다가 조금 어색한것 같아요!

단순히, 뷰에 보여주는 부분을 변경한다고해서 render-* 접두사를 써야한다고 생각해서 저런 요상한 네이밍이 나왔네요 ㅋㅋㅋㅠㅠ
근데 생각해보니 해당 함수가 렌더링된 아이템을 삭제해주는 작업을 해주기 때문에 deleteRenderedProduct 라고 수정해주었습니다 :D
감사합니다.

56293e9

wmakerjun · 2022-04-11T02:45:09Z

src/js/ui/snackbarUI.ts

+  }
+
+  open(type, message) {
+    if (this.$container.childElementCount >= 3) {


3은 어떤 의미를 가지는걸까요~? 🤔

스낵바가 여러개가 동시에 화면에 보여져야할 경우 최대 3개까지 보여지게 해놨기때문에 들어간 숫자인데 이렇게 놓으면....정말 코드를 읽는 입장에서는 뭐지??! 싶겠네요 ... 상수로 분리해주겠습니다 :D 감사합니다.

93b4899

wmakerjun · 2022-04-11T02:56:48Z

cypress/integration/authorization.spec.js

@@ -0,0 +1,110 @@
+const URL = "http://localhost:9000";


cypress-json을 활용해봐도 좋을것 같아요~!

감사합니다~ 훨씬 간결해졌네요 👍 👍 👍

f272fd4

wmakerjun · 2022-04-11T03:10:00Z

cypress/integration/authorization.spec.js

+    cy.get("#email").type("test@test.com");
+    cy.get("#password").type("Abcd1234!");
+    cy.get(".submit-button").click();


중복되는 부분을 재사용할 수 있게 리팩터링 한다면 테스트 코드의 가독성도 더 좋아질것 같아요~!

이 부분 cypress commands 사용해서 해결했습니다 :)
계속 반복해서 사용되는 signUp 과 login 을 커맨드로 빼놓으니까 너무 간결하네요
감사합니당 👍
b99dd1f

- 중복되는 부분 제거 (try-catch)

moonheekim0118 · 2022-04-12T18:48:16Z

안녕하세요 준 😄 정말 좋은 리뷰 감사드립니다!
말씀해주신 대로, 상수분리와 네이밍 관련해서 피드백 반영 완료했습니다 :D
코드를 다시 보다보니, 저번주에 급해서 놓쳤던 부분이 한 두군데가 아니네요 😭...코드리뷰 하시느라 정말 고생하셨습니다..큽..
cypress 에서도 Commands 를 사용해서 계속해서 반복되는 로직 (로그인,회원가입)을 줄였고, constants 를 정의해주었습니다 !

그럼에도 불구하고 아직 네이밍관련해서는 '더 좋은 네이밍' 에 대한 의문이 끊이질 않네요 😭 .. 준이 주신 피드백들 참고해서 앞으로 더 좋고 명확한 네이밍을 하도록 노력해보도록 하겠습니다 :D

혹시나 더 수정할 부분이 있거나, 궁금하신 점이 있다면 알려주시면 감사하겠습니다 🙇 🙇
다시 한번 좋은 리뷰 정말 감사드려요 👍 👍 👍 👍

wmakerjun · 2022-04-15T05:42:57Z

호프~ 정말 빠르게 피드백 반영해주고, 고민한 과정까지 상세하게 남겨줘서 소통하는 과정이 너무 큰 즐거움이었어요!
네이밍은 코치들 역시 계속해서 고민이 많이 되는 과정인데여 😭
읽기 좋은 네이밍은 혼자서 고민하기 보다는 주변 사람들과 피드백 주고 받을 때 확실히 더 명확한 아이디어가 많이 떠오르더라구요. 고민되는 네이밍이 있다면 다른 크루들과 의견 주고 받으면서 '어떤 과정을 통해 좋은 네이밍이 나올 수 있을까?'에 대한 호프만의 방법을 찾아나가면 너무 멋질것 같아요.

우테코 수료할 때 쯤엔 "네이밍은 여전히 어렵지만, 이렇게 접근하면 더 나은 네이밍을 만들 수 있는것 같아!"라는 호프만의 철학이 있으면 너무 눈부실것 같습니다 🤩

너무너무 고생했구요 레벨2에서 만날게요! 😆

moonheekim0118 added 28 commits April 3, 2022 13:22

docs: 상품 구매 기능목록 작성

781c65c

refactor: enum -> as const 로 수정

d59a471

refactor: changes-product관리 도메인 통합

09116be

feat: 상품 구매 탭 추가

a4e8bad

feat: 상품구매탭 초기 UI 구현

1a0a81d

feat: 돈 충전 기능 구현

5a0bc85

feat: 상품 구매 기능 구현

c287f2b

feat: 잔돈 반환 구현

3faf31d

feat: json-auth-server 추가

f0fce7b

feat: 페이지에 따라서 header 다르게 렌더링 되도록 수정

221decb

feat: 로그인,회원가입 기본 페이지 구현

a12004d

feat: signUp 기능 구현

6ed9ee5

feat: login 기능 구현

c81b604

feat: 페이지 진입 시, 유저 상태에 따라 헤더 뷰 변경 구현

eec7258

style: form css 추가

4ec2c12

feat: 로그인,회원가입 에러메시지 alert 추가

5fdb681

refactor: header template 생성

539d98c

refactor: 커스텀이벤트 타입 지정

9a83815

refactor: validation 분리

902ad3d

refactor: 반환타입 void 제거

2146149

feat: 로그아웃 기능 구현

b69f37c

feat: 로그아웃 기능 구현

10be023

fix: async-await 추가

fdc53ef

feat: 회원정보 수정 기능 구현

dbe5eab

feat: 스낵바 구현

8a7868e

style: 마크업 수정

ca51c06

refactor: break -> return 으로 수정

f6b4b6d

chore: npm script 추가

1e11d7a

moonheekim0118 changed the base branch from main to moonheekim0118 April 7, 2022 06:09

pocojang requested a review from HyeonaKwon April 7, 2022 06:27

fix: 누락된 타입 추가

d5ad3d0

wmakerjun requested changes Apr 11, 2022

View reviewed changes

moonheekim0118 added 23 commits April 11, 2022 21:24

fix: 불필요한 공백 제거

61fa29c

refactor: api request 함수 수정

bc1144d

- 중복되는 부분 제거 (try-catch)

refactor: 객체명 수정

7a364de

refactor: isLoggedIn -> getLoggedInUser 메서드 명 수정

0b6e49a

refactor: 에러메시지 상수화

6028b23

refactor: 불필요한 return 제거

da318cd

refactor: 불필요한 연산 제거

fa96180

refactor: 상수 추가

7e981e7

refactor: input-> inputCollection 으로 이름 수정

08053b2

refactor: validation 함수 통합

f6d0fa1

refactor: checkValid->validate 로 함수명 수정

7dd00c4

refactor: 누락된 상수 추가

ef46977

refactor: validateProduct 함수 작성

e28e283

refactor: 변수명 수정

dd3760e

refactor: coinTypes 상수로 분리

1b678d8

refactor: 메서드명 수정

39101df

refactor: updateProduct 인자 객체로 수정

9716133

refactor: 메서드명 수정

56293e9

refactor: 스낵바 최대스택 상수화

93b4899

refactor: cypress baseUrl 설정

f272fd4

fix: 오타 수정

210e578

refactor: cypress command 추가

b99dd1f

refactor: cypress uitl 분리

37ca5d1

fix: 파일명 수정

f51028b

wmakerjun approved these changes Apr 15, 2022

View reviewed changes

wmakerjun merged commit d48dfa8 into woowacourse:moonheekim0118 Apr 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[2단계 - 자판기] 호프(김문희) 미션 제출합니다. #61

[2단계 - 자판기] 호프(김문희) 미션 제출합니다. #61

moonheekim0118 commented Apr 7, 2022 •

edited

Loading

wmakerjun left a comment

wmakerjun Apr 8, 2022

wmakerjun Apr 8, 2022

wmakerjun Apr 8, 2022

moonheekim0118 Apr 11, 2022

wmakerjun Apr 15, 2022

wmakerjun Apr 10, 2022

moonheekim0118 Apr 11, 2022

wmakerjun Apr 10, 2022

moonheekim0118 Apr 11, 2022

wmakerjun Apr 11, 2022

moonheekim0118 Apr 12, 2022

wmakerjun Apr 11, 2022

moonheekim0118 Apr 12, 2022

wmakerjun Apr 11, 2022

moonheekim0118 Apr 12, 2022

wmakerjun Apr 11, 2022

moonheekim0118 Apr 12, 2022

wmakerjun Apr 11, 2022

moonheekim0118 Apr 12, 2022

moonheekim0118 commented Apr 12, 2022

wmakerjun commented Apr 15, 2022

[2단계 - 자판기] 호프(김문희) 미션 제출합니다. #61

[2단계 - 자판기] 호프(김문희) 미션 제출합니다. #61

Conversation

moonheekim0118 commented Apr 7, 2022 • edited Loading

로컬에서 실행하는 법

enum 을 사용하면 안되는 이유

readonly 타입이란?

as const 란?

wmakerjun left a 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

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

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

moonheekim0118 commented Apr 12, 2022

wmakerjun commented Apr 15, 2022

moonheekim0118 commented Apr 7, 2022 •

edited

Loading