[2단계 - 자주 가는 음식점] 치코(이재민) 미션 제출합니다. by jaeml06 · Pull Request #147 · woowacourse/javascript-lunch

jaeml06 · 2024-03-16T18:09:10Z

안녕하세요 블링! 이번 단계도 잘 부탁드립니다.

step2

https://jaeml06.github.io/javascript-lunch/

start

// 로컬 서버 실행

npm run start

// 테스트 실행

npm run test

컴포넌트 설명

새로 추가된 컴포넌트

starButton : 자주 가는 음식점을 추가하기 위한 이미지 버튼
categoryImage : step2에서는 restaurantDetail에서의 재사용을 위해 컴포넌트화
restaurantDetail: 음식점 상세 정보 내용을 보여주기 위한 modal contents
tabBar : 모든 음식점과 자주 가는 음식점을 보여주기 위한 tabBar

step2에서 주요 구현한 사항

모든 음식점, 자주 가는 음식점 선택하여 볼 수 있는 tabBar
- 탭의 종류 추가를 고려 하여 tabBar컴포넌트 배열을 매개변수로 받아 생성
- 자주 가는 음식점 클릭 시, 선택 되어있는 dropDown 값에 따라 음식점 리스트 보여줌
자주 가는 음식점을 추가하기 별 이미지 버튼 추가
- 빈별 클릭 시, 자주 가는 음식점 추가, localstorage 'favoriteRestaurants'에 저장
- 선택된 별 클릭 시, 자주 가는 음식점 삭제, localstorage 'favoriteRestaurants'에 삭제
- 자주 가는 음식점 탭에서 선택된 별 클릭 시, 바로 사용자 실수를 염두, 바로 리스트에 반영하지 않음
음식점 상세 페이지 모달 구현
- 모달에서 자주 가는 음식점 추가, 삭제 가능
- 삭제하기 버튼 클릭시 레스토랑 전체 리스트에서 삭제

이번 미션을 진행하며 고민한 점

AppControl의 필드와 메서드의 사용에 관하여

이번 미션을 진행하면서 restaurantManager을 필드로 가지고 있는 것이 적합하다고 판단하여 객체 형태에서 클래스 형태로 전환하였습니다.

이때 restaurantManager을 컴포넌트에서 필요로 하는 경우가 있습니다. 또는 updateRestaurantList() 함수를 컴포넌트 이벤트로 호출할 필요가 생겼습니다. 그래서 저는 고민을 해야했습니다. this와 restaurantManager 인자로 넘겨 줄지, 컴포넌트 생성 시 이벤트 콜백함수를 appcotrol에서 선언하여 콜백함수를 넘겨줄지 고민했습니다. 후자가 컴포넌트 생성에 클래스 값 자체에 의존하지 않는다고 생각하여 외부에서 선언된 콜백 함수를 넘겨주는 방식을 선택했습니다. 블링은 어떻게 생각하시나요?
id와 class의 사용

저는 개인적으로 class로 태크들을 구분합니다. 그 이유는 id를 사용할 경우 id는 고유 값이 되어야 하는데, 실제로 중복된 id를 사용해도 에러를 발생하지 않아 중복된 id가 있는지 알기 어렵기 때문입니다. 그래서 이번에 class를 사용했으나 실제 태크들을 구분하기에는 가독성이 좋지 않다고 느꼈습니다. 저는 이런 이유로 class를 사용할 생각이지만 실제로 id를 이용하여 작성하는 경우도 많은가요?
retaurant 객체에 자주 가는 음식점인지를 값으로 가지고 있기

자주 가는 음식점을 어떻게 저장하고 있어야하는지 고민이였습니다. 개인적으로 'localStorage'에 저장되어 있는 배열의 요소인 객체에는 자주 가는 음식점인지 저장되어 있는 것은 좋은 해결 방법이 아니라고 생각했습니다. 그 이유는 제 생각에 restaurant는 등록된 모든 음식점을 관리 하는 것인데 만약 사용자가 여러 명 있는 경우 각각의 사용자마다 전체 음식점 리스트에 자주 가는 음식점인지 아닌지 속성으로 가지고 있는 것은 합리적이지 않다고 생각했습니다. 그래서 저는 내부에 favoriteRestaurants 배열에 추가하여 저장하는 방식을 사용합니다. 그래서 현재 자주 가는 음식점인지 나타내기 위해 별의 이미지를 선택해야하는데 includes함수를 사용해서 favoriteRestaurants에 있는 요소인지 판별하여 별의 이미지를 결정합니다. 이 이외의 방법은 localStorage에서 객체의 배열을 받아 오는데 이 객체에 추가적으로 favorite이라는 boolean속성 값을 만들어 관리하고 localStorage에 업데이트할 때 다시 favorite이라는 속성을 제거하는 방식이 있습니다. 과연 블링은 어떤 방식으로 자주 가는 음식점인지를 관리할 것인가요? 저는 이것이 궁금합니다.

이번 진행하며 느낀점

이번 미션에서 처음으로 E2E테스를 작성했습니다. 개인적으로 만족한 경험이였습니다. 실제로 이번 프로젝트를 진행하면서 음식점을 추가하고 삭제하는 과정, 자주 가는 음식점을 추가하는 과정에서 코드를 수정하고 검증하는 과정에서 매번 일일이 실제 사이트를 이용해보며 검증하는 방식을 사용하다보니 개발 피로도가 높아진다는 느낌을 받았습니다. 프로젝트를 기능을 완성하고 E2E테스트를 작성하면서 자잘한 변경 사항이 생겼는데 개발자가 직접 데이터를 입력하지 않고 자동으로 핵심 기능을 검증하니 개발 시간을 줄일 수 있었다. 이번 미션은 프로젝트 완성 후, 이용했지만 다음 미션부터는 실제 기능을 구현하면서 도입하고 싶다. 물론 개발 시간이 촉박하다면 E2E테스트 직성을 어떻게 활용해야 할지 고민입니다.

step1에 대한 답변

저번 step1에서 타입 추론에 대한 이야기를 한 적이 있습니다. 여러 생각을 해봤고 고민했습니다.

저는 타입 추론으로 any가 나오는 경우를 제외하고 타입스크립트가 타입 추론이 가능한 경우 타입 선언을 하지 않는다. 그외 문제가 발생하는 경우에 타입 선언을 사용한다. 라고 일단 결론을 지었습니다.

다만 직접적인 경험이 부족한 관계로 앞으로 더 경험해보면서 다듬어야 할 것 같습니다. 블링의 기준은 어떤가요?

📦src
┣ 📂component
┃ ┣ 📂modal
┃ ┃ ┣ 📜addRestaurantModal.js
┃ ┃ ┗ 📜restaurantDetail.js
┃ ┣ 📂toast
┃ ┃ ┣ 📜toast.css
┃ ┃ ┗ 📜toast.js
┃ ┣ 📜button.js
┃ ┣ 📜categoryImage.js
┃ ┣ 📜dropDown.js
┃ ┣ 📜header.js
┃ ┣ 📜input.js
┃ ┣ 📜labelWrapper.js
┃ ┣ 📜modal.js
┃ ┣ 📜restaurantCard.js
┃ ┣ 📜starButton.js
┃ ┣ 📜tabBar.js
┃ ┗ 📜textArea.js
┣ 📂constant
┃ ┣ 📜appString.ts
┃ ┣ 📜error.ts
┃ ┗ 📜select.ts
┣ 📂domain
┃ ┗ 📜RestaurantManager.ts
┣ 📂images
┃ ┣ 📜add-button.png
┃ ┣ 📜category-asian.png
┃ ┣ 📜category-chinese.png
┃ ┣ 📜category-etc.png
┃ ┣ 📜category-japanese.png
┃ ┣ 📜category-korean.png
┃ ┣ 📜category-western.png
┃ ┣ 📜favorite-icon-filled.png
┃ ┗ 📜favorite-icon-lined.png
┣ 📂interface
┃ ┗ 📜Restaurant.ts
┣ 📂utils
┃ ┗ 📜selector.js
┣ 📂web
┃ ┗ 📜AppControl.js
┗ 📜index.js

totalRestaurants배열에서 입력 받은 레스토랑 제거

jaeml06 · 2024-03-16T18:51:04Z

-
-  constructor(restaurants: Restaurant[] = []) {
-    this.restaurants = [...restaurants];
+  private favoriteRestaurants: Restaurant[];


본문에 favoriteRestaurants 관리에 관한 질문입니다.

uk960214

안녕하세요 치코!
이번 단계 미션도 잘 봤습니다! 역시 이전 단계에서 어느정도 틀을 잘 잡아두니
깔끔하게 코드가 나온 것 같다는 생각이 들었습니다.
전체적으로 크게 손 볼 곳은 없고 간단한 코멘트만 몇 개 남겼습니다!

AppControl 관련

저도 지금 방식처럼 콜백을 넘겨서 사용처에서 인스턴스에 직접 접근하지 않는 방식이 더 적절하다고 생각합니다!

id와 class

말씀하신대로 class를 사용하는 경우가 더 많은 것 같아요. 정말 불가피한 경우에만 id를 사용하게 되지 않나 생각합니다. 혹은 리액트에서 app div처럼 하나 확실한 값이 있을 것이 보장되는 경우거나요!

자주 가는 음식점

지금 방식도 크게 이상하다고 생각하지는 않는데요, 제 경험상 실제 서버와 작업하게 되는 경우 말씀하신 두 가지 방식을 혼합해서 사용하는 것 같아요.
우선 사용자가 여러명 있는 경우는 사실 크게 상관이 없습니다! 로그인을 하는 방식이 마련되어있고 서버에 그 정보를 넘겨야한다면 서버에서 해당 사용자를 식별하고 like 여부를 판단해서 보내줄테니까요.
두 가지 방식을 혼합할 수 밖에 없는 이유는 서버에 얼마나 많은 데이터가 있을지를 확신할 수 없기 때문이 가장 큽니다. 실제로 레스토랑의 목록과 즐겨찾기 한 목록의 길이가 얼마가 될지 모르는데, 예를 들어 레스토랑은 100만개, 즐겨찾기가 1000개가 된다면 이를 연관 없는 두개의 값으로 각각 들고와서 비교해서 렌더링하기는 쉽지 않겠죠. 그래서 전체목록에도 해당 사용자가 like했는지 여부가 포함되어야 하겠죠. 또 전체 목록에 like를 가지고 즐겨찾기 목록을 페이징해서 불러오기는 어려움이 마찬가지로 있을 것입니다. 그래서 현실적으로는 두 가지를 모두 사용하지 않을까 싶습니다!

E2E

유용성을 느끼셨다니 다행입니다! E2E는 갖추기만 하면 정말 쓸모가 많긴 하겠죠. 대신 확실히 구축하는데에 어려움을 느끼긴 합니다. UI는 더구나 자주 변하고 그럴 때마다 테스트도 수정해주어야 하는 것이니까요. 아마 그래서 잘 변하지 않는 서비스의 가장 핵심적인 플로우를 위주로 테스트를 구축하는 것이 좋지 않을까 생각합니다.

타입 추론

많이 고민 해주셔서 감사합니다! 저는 전에 말씀 드린 것처럼 추론되는 타입보다 더 좁은 타입으로 선언하고 싶을 때 위주로 타입 선언을 하는 편입니다.

추가적인 내용

전반적으로 오타가 많은 것 같아요. vscode에는 스펠링 체크를 해주는 익스텐션도 있는데요, lint 오류처럼 혹은 워드에서 잘못된 단어를 입력했을 때처럼 밑줄로 표시해주는데, 이런 툴을 사용해봐도 유용할 것 같습니다!

사용하지 않는 import, 함수들은 지우면 코드가 더 깔끔해질 것 같습니다. eslint 룰로 사용하지 않는 import 문에 대해서 경고를 띄워주고, 코드 베이스 전반에서 reference count를 보여주는 extension을 사용해도 좋을 것 같아요!

star 버튼이 정렬이 살짝 어긋난 것 같은데 요것도 수정 가능할까요??

uk960214 · 2024-03-17T00:16:05Z

    const sortedByAscendingName: RestaurantData[] = sortingName.output;
    // then
-    expect(restaurantManager.sortByAscendingName()).to.eql(
+    expect(restaurantManager.getUpdatedTotalRsetaurants()).to.eql(


오타가 있네요!

Suggested change

expect(restaurantManager.getUpdatedTotalRsetaurants()).to.eql(

expect(restaurantManager.getUpdatedTotalRestaurants()).to.eql(

넵 수정했습니다

uk960214 · 2024-03-17T00:25:29Z

+      if (tabItem.className.includes('tab__bar__item')) {
+        $('.tab__bar__item__checked').className = 'tab__bar__item';
+        tabItem.className = 'tab__bar__item__checked';
+      }


toggle을 사용하면 조금 더 간단하게 정리할 수도 있을 것 같아요!

toggle을 사용하여 변경하였습니다. 다만 개인적으로 큰차이는 없었습니다. 이유는 tabBar는 무조건 하나는 선택되어야 하기 때문입니다. 그래서 개인적으로 차이는 느끼지 못했습니다.

저는 toggle을 사용하면서 체크 여부 클래스도 분리된 것도 좋다고 생각합니다!
더불어서 문자열로만 바꾸는 것보다, toggle을 통해서 ON/OFF를 변경해주는 것도 이해하기 훨씬 쉽다고 생각하구요.
하지만 차이를 느끼지 못하신다면 그정도는 취향차이로 볼 수 있을 것 같습니다!

죄송합니다. 제가 말을 잘 이해 못했습니다. checked라는 클래스 명을 쓰지말고 ON/OFF라는 클래스명을 쓰라는 말인가요? 또한 체크 여부도 함수도 분리된 것이 좋다는게 무슨 말이가요?

앗 아니요! toggle 자체가 클래스를 껐다 켰다하는 것과 유사하게 동작하는 것을 비유한 것이었어요!
그러니까

checked를 별도의 클래스로 분리해서

element.className = 'tab__bar__item__checked' 보다 element.classList.toggle('checked')
로 관리하는 편이 이해하기 더 쉽지 않을까 하는 말이었습니다!

uk960214 · 2024-03-17T00:41:08Z

+    switch (this.curentSelectedSorting) {
+      case '이름순':
+        return this.sortByAscendingName([...this.favoriteRestaurants]);
+      case '거리순':
+        return this.sortByAscendingWalkingTime([...this.favoriteRestaurants]);
+    }
+    return [];


switch 문을 쓸 때는 default case를 항상 추가해주는 게 좋은데,
마지막 return 대신에 default를 return []으로 해줘도 될 것 같은데 어떻게 생각하시나요?

RestaurantManager.ts상에 존재하는 switch문에 default case를 추가 했습니다

uk960214 · 2024-03-17T01:01:18Z

-          this.updateRestaurantList(
-            restaurantManager.filteredRestaurants()
-          );
+          this.#restaurantManager.udateCurentCategoty(category);


이 매서드 네이밍에도 오타가 있네요

Suggested change

this.#restaurantManager.udateCurentCategoty(category);

this.#restaurantManager.updateCurrentCategory(category);

넵 수정했습니다.

uk960214 · 2024-03-17T01:01:46Z

+    }
+  }
+
+  curentTabRestaurantList() {


Suggested change

curentTabRestaurantList() {

currentTabRestaurantList() {

넵 수정했습니다

uk960214 · 2024-03-17T01:12:20Z

+  addFavoriteRestaurant(restaurant) {
+    this.#restaurantManager.addFavoriteRestaurant(restaurant);
+  }
+
+  removeFavoriteRestaurant(restaurant) {
+    this.#restaurantManager.removeFavoriteRestaurant(restaurant);
+  }


이 함수들은 호출되는 곳이 없는 것 같은데 확인 부탁드립니다!

이부분도 삭제 했습니다

uk960214 · 2024-03-17T01:13:33Z

  getRestaurants(): Restaurant[] {
-    return [...this.restaurants];
+    return [...this.totalRestaurants];
  }


이 함수도 더이상 사용중인 곳이 없는 것 같아요!

넵 삭제했습니다.

uk960214 · 2024-03-17T01:14:30Z

+import createButton from '../button';
+import modal from '../modal';
+import { createStarButton } from '../starButton';
+import { KOREAN_CATEGORY } from '../../constant/select';


이 import는 더 이상 사용되지 않는 것 같아요!

전체적으로 더 이상 사용하지 않은 import를 삭제했습니다

uk960214 · 2024-03-17T01:18:41Z

+    }
+  }
+
+  changeStarEvent(event, restaurant) {


changeStarEvent라는 이름이 적절한지 잘 모르겠네요!
실제 favorite을 클릭하는 시점에서 여기까지 like, favorite라는 네이밍을 함수에서 찾아보기가 어려운데요
만약 이 함수는 정말 단순하게 '별 모양 UI를 변경한다'는 의미라면
중간에 도메인 로직을 명확하게 알 수 있는 'clickFavoriteButton'같은 네이밍이 있어야 코드를 파악하기 더 좋을 것 같아요.
이 때는 이 즐겨찾기가 별이 될지, 하트가 될지, 체크가 될지는 모르니 UI에 대한 네이밍은 제거하구요!

무슨 말씀인지 이해했습니다. 확실히 UI에 대한 네이밍은 제거하는 것이 UI 변경시 더 유연하게 대처할 수 있겠네요

또한 starButton이라는 컴포넌트로 favoriteButton으로 변경하였습니다.

uk960214 · 2024-03-17T01:19:19Z

+Cypress.Commands.add('visitMain', () => {
+  cy.visit('http://localhost:8080/');
+});
+
+Cypress.Commands.add('setRsetaurants', (restaurants) => {
+  window.localStorage.setItem('restaurants', JSON.stringify(restaurants));
+});
+
+Cypress.Commands.add('openAddModal', () => {
+  cy.get('.gnb__button').click();
+});
+


커맨드 사용 아주 좋습니다!

감사합니다

jaeml06 · 2024-03-17T07:26:39Z

답변 감사드립니다.
1.전반적으로 오타가 많은 것 같아요.
제가 생각한 것보다 오타가 훨씬 많았네요. 죄송합니다. 이번에 추천해주신 spell check 익스텐션을 설치해서 오타를 수정했습니다. 추천 감사합니다.

넵 사용하지 않은 import문도 삭제 했습니다. 이번에 reference count를 보여주는 설정을 켜 확인 했습니다.
넵 음식점 설명에 따라 favorite버튼의 위치가 달라졌는데 위치가 일관적으로 적용되게 수정하였습니다.

uk960214

빠르게 반영해주셔서 감사합니다 치코!
전반적인 작업은 마무리 된 것 같고, 사소한 코멘트들 위주로 추가적으로 남겨보았는데요,
시간 되시는만큼 고민하고 도전해주시고 다시 리뷰 요청 주시면 그 때 머지하겠습니다!

uk960214 · 2024-03-18T03:35:48Z

+  const starButton = render({name, favoriteRestaurantNames});
+  if (favoriteCallback) starButton.addEventListener('click', (event) => {
+    favoriteCallback(event);
+    });
+  return starButton;


star라는 네이밍이 함수명에서는 제외됐지만 내부 변수에는 남아있네요! 요것도 함께 처리해주면 좋을듯 합니다.

아 이걸 확인 못했는데 같이 처리하겠습니다.

uk960214 · 2024-03-18T03:37:37Z

 import modal from './modal.js';

-function createHeader({ className, left, right, restaurantManager }) {
+function createHeader({ className, left, right, RestaurantAdditionCallback }) {


콜백함수의 네이밍이 더 명확해진 것은 좋지만 일반 함수인데 파스칼 케이스보다는 카멜 케이스가 더 적절할 것으로 보입니다!

넵. 주의 깊게 봤어야 했는데 마음만 급했나보네요. 수정하겠습니다.

uk960214 · 2024-03-18T03:44:55Z

+      default:
+        return [];


default 일 때는 아무 목록도 보여주지 않는 것보다 기본 선택이라고 생각되는 정렬의 목록이라도 보여주는 게 낫지 않나라는 생각이 드네요!

확실히 그게 보편적인 방법이겠네요. 제가 빈 배열을 반환하면 사용자는 아예 서비스를 이용에 문제가 생기는데 기본 선택 정렬 목록이라도 보여주면 사용자 이용에 불편은 있어도 서비스를 이용하지 못한다는 최악의 상황은 막을 수 있으니요

uk960214 · 2024-03-18T03:45:58Z

+    localStorage.setItem(
+      'restaurants',
+      JSON.stringify(this.favoriteRestaurants)
+    );
+    return [...this.favoriteRestaurants];


여기는 totalRestaurants가 저장되고 반환되어야 할 것 같은데 아닌가요??

으악! 제가 중요한 실수로 놓치고 그냥 지나쳤네요... 수정했습니다. 이와 같은 오류를 e2e테스트에서도 못찾았으니 '테스트 코드를 더 자세하게 작성했어야 했나?' 라는 생각이 듭니다.
E2E 테스트는 핵심적인 기능을 주로 테스트 한다고 생각했는데 무엇을 테스트 목록에 두어야할지 더 고민해 봐야겠네요.

전에 공원이 주신 팁이 있는데 '오류가 발생해서 수정해야 했던 부분은 테스트 케이스로 남긴다'라는 부분이었어요!
지금처럼 어떤 검증 과정에서 오류가 난 것을 발견했다면, 그걸 테스트 케이스에 넣으면 발견하기 어려운 엣지 케이스들에 대해서도 보완이 가능하다고 생각합니다!

uk960214 · 2024-03-18T03:46:50Z

+    localStorage.setItem(
+      'favoriteRestaurants',
+      JSON.stringify(this.favoriteRestaurants)
    );


localStorage.setItem()이 생각보다 여러번 호출되는 것 같은데, 호출의 길이도 길고 한번 공통화 해주는 것은 어떻게 생각하시나요?

#147 (comment)

uk960214 · 2024-03-18T03:49:55Z

+          callback: () => {
+            this.updateRestaurantList(
+              this.#restaurantManager.getUpdatedTotalRestaurants()
+            );
+            this.#currentTab = '모든 음식점';
+          },


이런 콜백함수는 따로 선언하고 여기서 넘겨주는 것이 의미가 더 명확하고, 변화에도 더 유연할 것 같은데 어떻게 생각하시나요?
callback: handleAllRestaurantTabClick이거나 혹은 아래랑 통합해서
callback: handleTabClick('all'), callback: handleTabClick('favorite') 같은 느낌도 괜찮을 것 같구요.

넵 따로 선언하는 것으로 수정해보았습니다.

uk960214 · 2024-03-18T03:58:48Z

+      if (tabItem.className.includes('tab__bar__item')) {
+        $('.tab__bar__item__checked').className = 'tab__bar__item';
+        tabItem.className = 'tab__bar__item__checked';
+      }


저는 toggle을 사용하면서 체크 여부 클래스도 분리된 것도 좋다고 생각합니다!
더불어서 문자열로만 바꾸는 것보다, toggle을 통해서 ON/OFF를 변경해주는 것도 이해하기 훨씬 쉽다고 생각하구요.
하지만 차이를 느끼지 못하신다면 그정도는 취향차이로 볼 수 있을 것 같습니다!

restaurantAdditionCallback으로 변경

jaeml06 · 2024-03-18T07:17:04Z

+  #updateLocalStorage(key: localKey, value: Restaurant[]) {
+    localStorage.setItem(key, JSON.stringify(value));
+  }


이런 식으로 수정하란 말씀이신건가요?

저는 RestaurantManager 클래스이고 private 매서드인 만큼 더 과감하게 이렇게 가도 무방하다고 생각합니다.
의미가 그렇게 어렵지도 않은데 코드 양은 어느정도 줄일 수 있으니까요

Suggested change

#updateLocalStorage(key: localKey, value: Restaurant[]) {

localStorage.setItem(key, JSON.stringify(value));

}

#updateTotalLocalStorage(value: Restaurant[]) {

localStorage.setItem('restaurants', JSON.stringify(value));

}

#updateFavoriteLocalStorage(value: Restaurant[]) {

localStorage.setItem('favoriteRestaurants', JSON.stringify(value));

}

jaeml06 · 2024-03-18T07:49:59Z

이번에는 전체적으로 주의 깊게 확인했더라면 하지 않았을 실수들이 많았네요. 이번주에 테크톡이 있는데 새로운 미션이 시작하기전에 미리 준비하고 싶다는 마음이 앞서 제대로 확인을 하지 못한 것 같습니다.

uk960214

수고 많으셨습니다! 테코톡 준비와 병행하시려면 쉽지 않으셨겠네요 ㅜㅜ

기능적으로, 구조적으로 훌륭하게 미션을 구현해주셨습니다.
마음이 급하다보니 오타나, 컨벤션이 통일되지 않은 것, 코드가 정리되지 못한 부분들이 좀 있었는데요,
물론 충분히 이해는 되지만 앞으로는 여러 도구들을 써서 미연에 방지하면
바쁜 중에도 코드 퀄리티를 보장하는데 도움이 되지 않을까 생각합니다!

고생 많으셨고 테코톡도 성공적으로 마무리하시길 응원하겠습니다!

uk960214 · 2024-03-18T12:05:12Z

+  #updateLocalStorage(key: localKey, value: Restaurant[]) {
+    localStorage.setItem(key, JSON.stringify(value));
+  }


저는 RestaurantManager 클래스이고 private 매서드인 만큼 더 과감하게 이렇게 가도 무방하다고 생각합니다.
의미가 그렇게 어렵지도 않은데 코드 양은 어느정도 줄일 수 있으니까요

Suggested change

#updateLocalStorage(key: localKey, value: Restaurant[]) {

localStorage.setItem(key, JSON.stringify(value));

}

#updateTotalLocalStorage(value: Restaurant[]) {

localStorage.setItem('restaurants', JSON.stringify(value));

}

#updateFavoriteLocalStorage(value: Restaurant[]) {

localStorage.setItem('favoriteRestaurants', JSON.stringify(value));

}

uk960214 · 2024-03-18T12:08:26Z

+    localStorage.setItem(
+      'restaurants',
+      JSON.stringify(this.favoriteRestaurants)
+    );
+    return [...this.favoriteRestaurants];


전에 공원이 주신 팁이 있는데 '오류가 발생해서 수정해야 했던 부분은 테스트 케이스로 남긴다'라는 부분이었어요!
지금처럼 어떤 검증 과정에서 오류가 난 것을 발견했다면, 그걸 테스트 케이스에 넣으면 발견하기 어려운 엣지 케이스들에 대해서도 보완이 가능하다고 생각합니다!

uk960214 · 2024-03-18T12:20:02Z

+      if (tabItem.className.includes('tab__bar__item')) {
+        $('.tab__bar__item__checked').className = 'tab__bar__item';
+        tabItem.className = 'tab__bar__item__checked';
+      }


앗 아니요! toggle 자체가 클래스를 껐다 켰다하는 것과 유사하게 동작하는 것을 비유한 것이었어요!
그러니까

checked를 별도의 클래스로 분리해서

element.className = 'tab__bar__item__checked' 보다 element.classList.toggle('checked')
로 관리하는 편이 이해하기 더 쉽지 않을까 하는 말이었습니다!

jaeml06 added 28 commits March 13, 2024 11:01

docs: step2 기능 명세서 추가

565dbc9

docs: 컴포넌트 UI 목록

57f6285

feat: 자주가는 음식점 추가 버튼 UI 구현

9dab0d9

test: 자주 가능 음식집 추가 삭제 테스트

8a37c1c

fix: 테스트 케이스 수정

814589a

refactor: 자주 가는 음식점 추가 삭제 구현

0b84b82

feat: queryselectAll 유틸 구현

3be9e6d

feat: tabBar UI 구현

187c475

refactor: 도메인 로직 변겨으로 인한 호출 함수 변경

4d96532

feat: favoriteRestaurants배열 localStorage에 추가

0d13fd3

feat: favoriteRestaurantNames에 따라 별 체크 여부 변경

2223fba

refactor: appController 구조 변경

e139634

refactor: 이미지 박스 컴포넌트 구현

aeb11e8

refactor: 별 버튼 컴포넌트 구현

bd4fc41

refactor: createHeder 매개변수 변경

7fadc6a

refactor: modal 폴더 위치 변경

e31cfc0

feat: 레스토랑 상세정보 modal 내용 구현

cb8d082

refactor: RestaurantCard 매개변수 변경

6cb200e

feat: querySelectorAll util로 구현

f579bd7

feat: removeTotalRestaurant 구현

b76c6c8

totalRestaurants배열에서 입력 받은 레스토랑 제거

feat: restaurantCard 클릭시 모달 생성 구현

59b5397

refactor: tabBar 컨밴션 준수

0d7d5fe

refactor: css파일 분리

8700592

refactor: 컵벤션 준수

48ea247

refactor: css 파일 분리

800851b

test: e2e 테스트 작성

6b8e082

refactor: chageStar 이름 변경

271c71b

docs: 구현 항목 최신화

7340f15

jaeml06 commented Mar 16, 2024

View reviewed changes

uk960214 requested changes Mar 17, 2024

View reviewed changes

jaeml06 added 11 commits March 17, 2024 15:26

refactor: 변수및 함수명 맞춤법 준수

b3454c6

refactor: 변수및 함수명 맞춤법 준수

5122278

refactor: toggle함수 사용

ab70795

refactor: switch문 default case 추가

f952086

refactor: 불필요한 index.html 삭제

b64132a

refactor: 불필요한 import 삭제

382200b

refactor: changeStarEvent명 수정

6aa4065

refactor: starButton 파일 변경

a28ade5

refactor: 불필요한 함수 삭제

94311bf

refactor: restaurantListItemEvent함수 분할

c53809b

refactor: favoriteButton 정렬 변경

11b7cde

uk960214 requested changes Mar 18, 2024

View reviewed changes

jaeml06 added 4 commits March 18, 2024 14:45

refactor: starButton 변수 명 변경

50f7e17

refactor: RestaurantAdditionCallback함수명 변경

bb62763

restaurantAdditionCallback으로 변경

feat: switch문 default가 빈 배열을 반환하지 않음

1b7f579

refactor: localStorage.setItem 함수로 분리

782cce7

jaeml06 commented Mar 18, 2024

View reviewed changes

refactor: 콜백 함수 선언 분리

46be837

uk960214 approved these changes Mar 18, 2024

View reviewed changes

uk960214 merged commit 76da2de into woowacourse:jaeml06 Mar 18, 2024

	expect(restaurantManager.getUpdatedTotalRsetaurants()).to.eql(
	expect(restaurantManager.getUpdatedTotalRestaurants()).to.eql(

	this.#restaurantManager.udateCurentCategoty(category);
	this.#restaurantManager.updateCurrentCategory(category);

-  #updateLocalStorage(key: localKey, value: Restaurant[]) {
-    localStorage.setItem(key, JSON.stringify(value));
-  }
+  #updateTotalLocalStorage(value: Restaurant[]) {
+    localStorage.setItem('restaurants', JSON.stringify(value));
+  }
+  #updateFavoriteLocalStorage(value: Restaurant[]) {
+    localStorage.setItem('favoriteRestaurants', JSON.stringify(value));
+  }

Conversation

jaeml06 commented Mar 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

step2

start

컴포넌트 설명

새로 추가된 컴포넌트

step2에서 주요 구현한 사항

이번 미션을 진행하며 고민한 점

AppControl의 필드와 메서드의 사용에 관하여

id와 class의 사용

retaurant 객체에 자주 가는 음식점인지를 값으로 가지고 있기

이번 진행하며 느낀점

step1에 대한 답변

Uh oh!

Choose a reason for hiding this comment

Uh oh!

uk960214 left a comment • 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!

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!

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!

jaeml06 commented Mar 17, 2024

Uh oh!

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

jaeml06 commented Mar 16, 2024 •

edited

Loading

uk960214 left a comment •

edited

Loading