💻 리팩터링 리뷰

✒️ 리팩터링 리뷰에 앞서

위 글은 이전 글을 한 화면에 켜놓고 비교하면서 보면 좋을 것 같다.

리팩터링 리뷰를 진행하기에 앞서 어떤 방식으로 진행할지 설명하도록 하겠다. 먼저 리팩터링 설계 단계에서 리팩터링을 어떤 방향으로 진행했는지 살펴보도록 하며, 코드 리팩터링을 전체적인 변화 에서 먼저 살펴 보고 프로젝트 소개 에서 작성한 구현 기능 목록 단위로 기능별 코드 리팩터링 리뷰를 살펴보도록하겠다.

마지막으로 코드 리뷰 체크리스트 의 요구사항에 얼마나 부합하는 코드였는지 하나씩 체크하며 살펴보도록 하겠다.

---

📃 리팩터링 설계

먼저 아래의 사진을 보고 설명하도록 하겠다.

먼저 리팩터링 요구사항을 작성했다. 이건 코드 리뷰 체크리스트의 요구사항에 따라 작성하고 몇가지를 추가해줬다. 그러고나서는 구현 기능 코드 별로 리팩터링을 해줘야겠다고 생각하고 구현 기능 코드 리팩터링을 작성해줬다. 마지막으로 리팩터링이 끝나면 에러나 오류가 발생하는 부분을 해결해주기위해 구현할 기능 목록을 작성해주었다.

설계한 내용을 리포지토리의 README.md 파일에 마크다운으로 작성해주었다.

---

🕵️ 코드 리팩터링

🗃️ 전체적인 변화

커밋메시지

이전 글을 보고 왔다면 알겠지만, 리팩터링을 진행하면서는 커밋메시지를 AngularJS Commit Message Conventions 에 맞게 작성해주었다. 이제 이전 글에서 본 끔찍한 커밋메시지는 존재하지 않는다.

디렉토리

JS 파일을 일단은 폴더 별로 분류해주었다. 이전 글에서 밝혔듯이 아직까진 디렉토리를 어떻게 나눠야하는지 명확히 모른다. (우테코에서 배우지 않을까 싶다)
그래서 내가 보기에는 각자 역할에 따라 분류가 잘 되어있는 것 같다.

코드 컨벤션

ESlint와 Prettier를 활용하여 Airbnb JavaScript 스타일 가이드 한국어 번역본에 맞게 코드를 작성해주었다.

모듈화

이전 글에서 밝히지는 않았던 것 같지만, 리팩토링 전에는 import문을 한 번도 활용하지 않았고, 모듈화 없이 모든 javascript 파일을 HTML 파일마다 script 태그를 이용해서 작성해주었다.

대충 이런 식으로 모든 HTML파일에 script태그들을 때려박았었다..

리팩터링 후에는는 index.js 파일만을 script 태그를 활용해 불러오고 index.js 내부에 있는 App 내부에서 모든 활동을 처리하도록 해주었다. 나는 다른 것보다 이렇게 모듈화를 해준 부분이 리팩터링에서 가장 큰 부분이었다고 생각한다.

이제 전체적인 부분을 모두 살펴봤으니 저번 글에서 봤던 순서대로 기능별 코드 리팩터링 리뷰를 진행 해보도록 하자.

---

📁 기능별 코드 리팩터링 리뷰

• Home 화면에서 화면을 드래그하면 섹션이 위 아래로 드래그 되도록 구현.

//scrollEvent.js

import { X, DELTA_Y_BOUNDARY, FIRST_PAGE } from '../constants/constant.js';
import { $ } from '../utils/dom.js';

export const handleScroll = (e, currentPage) => {
  if ($('.home-header') === null) {
    return;
  }
  const lastPage = document.querySelectorAll('.section').length;

  if (e.deltaY > DELTA_Y_BOUNDARY && currentPage < lastPage) {
    currentPage++;
  } else if (e.deltaY < DELTA_Y_BOUNDARY && currentPage > FIRST_PAGE) {
    currentPage--;
  }

  scrollScreen(currentPage);
  return currentPage;
};

export const scrollScreen = currentPage => {
  if ($('.menu-tree').classList.contains('is-active')) {
    return;
  }
  const pageHeight = window.innerHeight;
  window.scrollTo({
    top: currentPage * pageHeight,
    left: X,
    behavior: 'smooth',
  });
};

• 제일 먼저 보이는 점은 전역변수와 매직넘버 가 사라졌다.

• 그리고 이벤트를 처리하는 비즈니스 로직인 handleScroll 과 화면을 스크롤 해주는 UI로직인 scrollScreen 이 나뉘어졌다.

• 이제 함수는 하나의 기능만을 하는 작은 함수 이며, 우테코 프리코스 요구사항이었던 15줄 이내이기도 하다.

• 그리고 오류가 발생할 수 있는 추가적인 상황을 추가적인 if문으로 걸어내주는 부분이 추가되어졌다.

• if, else if 문에 존재했던 중복된 코드도 이전 글의 코드와 비교해보면 제거되었음을 알 수 있다.

• 함수선언문으로 선언된 함수들이 모두 함수표현식으로 바뀌었다.

아직까지 못 고친 오류가 있다..
아직까지 못 고친 부분은 위에 있는 구현할 기능 목록 에서 살펴볼 수 있다.

---

• header 에 커서를 올리면 header 가 흰 색으로, header 내부 아이콘들이 검은 색으로 변하도록 구현.

• 메뉴 아이콘 을 클릭할 시 상태에 맞게 메뉴가 열리고 닫히도록 구현.

• 화면 좌측 상단의 메뉴 아이콘 을 클릭할 시 메뉴 아이콘 의 모양이 상태에 따라 ☰, X 로 변하는 애니메이션이 재생되도록 구현.

//clickEvent.js

import { $ } from '../utils/dom.js';

export const handleClick = e => {
  switch (e.target.id) {
    case 'menu-icon':
      menuTreeToggle();
  }
};

const menuTreeToggle = () => {
  const menuTree = document.querySelectorAll('.menu-tree-class-for-toggle');
  $('header').classList.toggle('is-active');
  for (let i = 0; i < menuTree.length; i++) {
    menuTree[i].classList.toggle('is-active');
  }
};

• wholeMenuTreeToggle을 MenuTreeToggle로 바꿔주었다.

• 아까와 같은 방식으로 비즈니스 로직 과 UI 로직 을 구분해주었다.

깜빡하고 for문의 초기식,조건식,증감식을 고쳐주지 않았다...

---

• resize 이벤트 발생 시, 화면사이즈에 맞게 배경사진 사이즈를 바꿔줌

없어도 내 의도대로 잘 작동해서 아예 제거해주었다.

---

• 새로고침 이벤트 발생 시, 섹션 페이지가 1번 페이지로 드래그 되도록 구현.

//loadEvent.js

import { $ } from '../utils/dom.js';
import { X, Y, DELAY_MILLISECOND } from '../constants/constant.js';

export const reloadScrollScreen = () => {
  const pageHeight = window.innerHeight;
  const stackBurgetAnimation = $('.stack-burger-animation4');

  setTimeout(() => {
    window.scrollTo(X, Y);
  }, DELAY_MILLISECOND);
  if (stackBurgetAnimation === null) {
    return;
  }
  stackBurgetAnimation.addEventListener('animationend', () => {
    window.scrollTo({ top: pageHeight, left: X, behavior: 'smooth' });
  });
};

• console.log를 제거해주었다.

• 모든 매직넘버 가 제거 되고, 상수로 바꿔주었다.

• 함수선언문 대신 함수표현식 으로 바꿔주었다.

• 이 기능에서 핵심적인 리팩터링은 버거킹 애니메이션이 끝나고 화면을 스크롤 해주는 것이었는데, 원래는 애니메이션 시간을 계산해서 setTimeOut을 사용해 화면을 기다렸다가 스크롤 했으나, 이제 animationend 라는 이벤트를 통해 애니메이션이 끝나면 화면을 스크롤 해준다.
  훨씬 상식적이다.

---

📋 체크리스트 확인

• ✔️ README.md 파일에 프로그램의 구현할 기능들을 분리해 작은 단위로 작성했는가?

• ✔️ 프로그램을 작성하면서 예외 케이스 등을 추가하는 등, README.md 파일을 업데이트했는가?

• ✔️ 프로그램 전체에 동일한 코딩 컨벤션을 정해놓고 이를 사용했는가?

• ✔️ 각각의 커밋이 어떤 커밋인지 알아볼 수 있도록 커밋메세지를 작성했는가?

• ✔️ 변수명 등을 통일성 있게, 축약 또는 생략하지 않고 작성했는가?

• ✔️ 어떤 프로그램인지 프로그램을 소개하는 내용을 README.md 파일에 작성했는가?

• ✔️ 값을 하드코딩하지 않고 상수를 활용하며, 해당 상수에 의도를 드러낼 수 있는 이름을 부여했는가?

• ❌ JavaScript에서 제공하는 api를 활용하여 함수가 명백히 무슨 일을 하는 지 알 수 있도록 코드를 작성했는가?

• ✔️ 전역공간을 사용하지 않았는가?

• ✔️ var를 사용하지 않았는가?

• ✔️ 하나의 함수를 하나의 기능만을 하는 작은 단위로 구현했는가?

• ✔️ 중복된 코드가 없는가?

• ✔️ 불필요한 변수, 임시변수들을 제거해줬는가?

• ✔️ 비즈니스 로직과 UI로직을 분리했는가?

아직 아쉬운 점이 존재하는 것은 사실이지만 이전과 비교하면 훨씬 코드가 보기에도 좋아졌고, 유지보수에도 좋아졌다! (사실 알아챈 사람은 알아챘겠지만 위에 강스포가 있었다)

---

✍ 글을 마치며

1차 리뷰는 이제 끝이 났다. 다음에 내가 성장함을 느끼면 새로운 프로젝트를 가져오던, 이 프로젝트를 또 다시 발전시키던 2차 리뷰로 다시 찾아오도록 하겠다.

긴 글을 읽어준 모두에게 감사하다는 말은 전하고 글을 마치도록 하겠다!