[코드잇] 위클리 미션 수행기 (5주차)

woolee의 기록보관소·2023년 4월 17일

코드잇부트캠프0기

목록 보기

7/24

지난 주(4주차) 코드리뷰 및 리팩토링 진행

코드 리뷰

4주차 위클리 미션에 대한 코드 리뷰를 받았다.

이번엔 셀프 코드 리뷰도 저번 주보다 더 적극적으로 진행했다.

내가 코드를 작성한 의도를 명확히 전달해야 멘토분께서도 수월하게 코드 리뷰를 진행할 수 있을 거라고 생각했기 때문이다. 그래서인지 코드 리뷰를 더 디테일하게 받은 것 같다.

이번 주에는 리팩토링에 시간을 엄청 쏟아야 할 것 같다.

리팩토링 목록 및 코드리뷰 정리

웹 컴포넌트를 만들 때는 고려할 점 - style @import를 사용하면 파일 분리가 가능하다 // 일반적으로 작게 만들 때는 inline으로 스타일을 넣어도 괜찮다

disconnectedCallback() 함수에서는 이벤트 핸들러를 삭제하기. 사용하지 않으면 GC가 안 돌아서 허덕일 수 있다

.inner 클래스를 부모 요소에 삽입하고 의미 없는 div.inner 요소는 남발하지 말고 삭제하기

카드 컴포넌트 배치할 때 9로 하드 코딩했는데, 그러지 말고 상수로 빼기 (상수는 PASCAL_CASE로 작성하기!)

불필요한 주석들 제거하기 (마저 지우지 못한 요소 등 전부 다)

컴포넌트마다 break point 정리하기. pc/table/mobile 이렇게 3개로만 압축하기

컴포넌트의 경우 js 파일만 사용하고 있으므로 Footer/app.js가 아니라 Footer.js로 변경하기

이중 삼항 연산자를 쓴 부분이 있는데, 유지보수 시 가독성이 떨어질 수 있으므로 if or switch로 변경하기

utils/validationUserPassword.js에서 !flag && userPassword && ... 이 부분이 길어지므로 의미 맥락으로 함수화하기

utils/validationUser.js에서 함수 바깥에 상수 배치하기

그리고 또 utils/validationUser.js 파일에서 else if가 길어지면 함수화해서 단순하게 만들기 그리고 type에 대한 조건만 있다면 switch 문으로 변경하는 게 더 가독성이 좋아보인다

리팩토링 진행 - disconnectedCallback 시 이벤트 리스너 제거 문제 해결

리팩토링 전 코드

함수를 위 그림과 같이 만들어두고 아래와 같이 코드를 작성했다.

this.shadowRoot.addEventListener(
      "click",
      (e) => {
        this.cardClickInteraction(파라미터...)
      }
    );

이 이벤트 리스너들은 connectedCallback에서 등록해주고
disconnectedCallback에서 제거해줘야 하는데, 익명함수라서 어떻게 지울지를 고민했다.

클로져를 사용해서 바로 익명함수를 리턴하고, 그 익명함수를 기명함수로 감싸주면 되지 않을까라고 생각했다.

클로져를 사용해서 event 객체를 파라미터로 받는 익명함수를 바로 리턴해주면, 콜백함수로 다른 파라미터들을 받을 수 있다고 생각했다.

이렇게 만들어주고 disconnectedCallback에서 이벤트리스너를 제거해주려 했는데, 제거되지 않는다.

이유를 추측해보자면, 결국 이런 구조를 써도 이벤트리스너에 등록되는 건 익명함수이므로 removeEventListener에서 찾을 수가 없는 것 같았다.

검색을 하다가 아래 링크에서 일단 해결할 수 있는 방법을 찾았다.
You’ve Got Options for Removing Event Listeners - Using AbortController()

이벤트 리스너에 등록해줄 때, 세번째 인자로 signal을 보내주고 disconnectedCallback에서 controller.abort()를 사용해서 시그널을 보낸 이벤트리스너들을 한번에 삭제해줬다.

abort()를 사용해서 생명주기가 끝날 때 한번에 제거해줬다.

<body>
    <div>태그</div>
    <p>태그111</p>

    <script>
      const test = document.querySelector("div");
      const ppp = document.querySelector("p");

      const controller = new AbortController();
      const { signal } = controller;

      const testCallback = (element) => {
        return function (e) {
          console.log(e);
          if (element) {
            console.log(element);
          }
        };
      };
      ppp.addEventListener("click", (e) => {
        console.log(e.target);
      });
      test.addEventListener("click", testCallback(test), { signal });

      controller.abort();
    </script>
  </body>

예를 들어 위와 같이 abort()를 실행해보면
signal을 보낸 이벤트리스너만 제거하고, 보내지 않은 이벤트리스너는 제거하지 않는 걸 볼 수 있다.

의도한 대로 잘 동작하는 것 같아서 일단 해결로 처리했다.

리팩토링 완료!

수정한 목록들

불필요한 주석들 제거하기 (실수로 전부 제거)
컴포넌트의 경우 js 파일만 사용하고 있으므로 Footer/app.js가
아니라 Footer.js로 변경하기
utils/validationUser.js에서 함수 바깥에 상수 배치하기
(전부 다 상수들은 파스칼 케이스로 변경했다)
(질문하기 : querySelector로 찾은 요소들도 파스칼 케이스를 사용해야 할까?)
.inner 클래스를 부모 요소에 삽입하고 의미 없는 div.inner 요소는
남발하지 말고 삭제하기
(셀프 코드 리뷰로 남기기 : container 역할을 하는 요소를 제외하고 나머지 의미없는 div.inner 요소는 제거하기)
disconnectedCallback() 함수에서는 이벤트 핸들러를 삭제하기.
사용하지 않으면 GC가 안 돌아서 허덕일 수 있다
(셀프 코드 리뷰 남기기 : abort()로 해결했는데, 더 좋은 방법이 있는지 궁금하다)
utils/validationUserPassword.js에서 !flag
&& userPassword && ... 이 부분이 길어지므로 의미 맥락으로
함수화하기
그리고 또 utils/validationUser.js 파일에서 else if가 길어지면
함수화해서 단순하게 만들기 그리고 type에 대한 조건만 있다면
switch 문으로 변경하는 게 더 가독성이 좋아보인다
이중 삼항 연산자를 쓴 부분이 있는데, 유지보수 시 가독성이 떨어질
수 있으므로 if or switch로 변경하기
이메일 오류 이후 비밀번호 focus 시 반복 alert 문제
컴포넌트마다 break point 정리하기. pc/table/mobile 이렇게
3개로만 압축하기

이번 주(5주차) 위클리 미션

디자인 시안 확인

5주차 위클리 미션 디자인 시안

요구사항 파악

지난 주 컴포넌트에 하드코딩했던 데이터들을 api에서 받아오는 걸로 변경하는 게 메인 요구사항이다.

크게 나눠 보면,

로그인 시 유저 데이터가 header에 대신 들어가야 하고
hero 콘텐츠에도 코드잇 대신 유저의 데이터가 들어가야 한다.
각각의 카드 컴포넌트의 내용도 api로 받아온 데이터가 들어가야 하며,
10 minutes ago라고 하드 코딩한 부분도 생성 시간과 현재 시간을 비교해 동적으로 계산할 수 있게 바꿔줘야 한다.

구현 (페어 프로그래밍)

이번 주 위클리 미션은 페어 프로그래밍으로 진행했다.

방식은 간단하게 다음과 같이 정하고 진행했다.

Driver와 Navigator로 역할을 나눠 진행했다.
뽀모도로 기법 (Pomodoro Technique)에 근거해, 시간 관리를 했다. 25-30분 간 페어 프로그래밍을 하고 5-10분간 휴식을 진행했다.

this.originalDate = new Date(props.createdAt);
this.createDate = this.originalDate.toLocaleDateString().slice(0, -1);
this.currentDate = new Date();
this.timeDiffMinutes = (this.currentDate - this.originalDate) / 1000 / 60;

this.prettyTimeDiff = (timeDiffMinutes) => {
  const r = (t) => Math.round(t);

  if (r(timeDiffMinutes) < 2) {
    return "1 minute ago";
  } else if (r(timeDiffMinutes) <= 59) {
    return `${r(timeDiffMinutes)} minutes ago`;
  } else if (r(timeDiffMinutes) <= 119) {
    return "1 hour ago";
  } else if (r(timeDiffMinutes / 60) <= 23) {
    return `${r(timeDiffMinutes / 60)} hours ago`;
  } else if (r(timeDiffMinutes / 60) <= 47) {
    return "1 day ago";
  } else if (r(timeDiffMinutes / 60 / 24) <= 30) {
    return `${r(timeDiffMinutes / 60 / 24)} days ago`;
  } else if (r(timeDiffMinutes / 60 / 24) <= 61) {
    return "1 month ago";
  } else if (r(timeDiffMinutes / 60 / 24 / 31) <= 11) {
    return `${r(timeDiffMinutes / 60 / 24 / 31)} months ago`;
  } else {
    return `${Math.floor(timeDiffMinutes / 60 / 24 / 31 / 12)} years ago`;
  }
};