본문 바로가기

Clean Code

[수업 중 코드리뷰] 우아한테크코스 프론트엔드 레벨1 총정리

 

 

우아한테크코스 과정에서 받고 있는 가장 값진 자산중에 하나인 피드백을 정리해보자.

다음은 크루들이 진행한 미션에 대해서 수업시간에 받은 공통 피드백이다. 우아한테크코스 프론트엔드 코치이신 @eastjun@devJang 께서 리뷰를 해주셨다.

( + 로또 미션 업데이트 완료, 2021. 03. 01. )
( + 유튜브 미션 업데이트 완료, 2021. 04. 07. )
( + 지하철 미션 업데이트 완료, 2021. 04. 10. )

 

 

계산기 미션 ➕➖

1. 주석에는 정말 필요한 내용만

주석을 작성하면 읽는 사람으로 하여금 더 많은 시간을 들이게 함을 명심하자.

<주석에 담아도 좋은 내용>

- 코드작성자의 통찰을 포함하기
// e.g. 이 데이터에서 이진트리는 해시테이블보다 40%정도 빠르다.
- 결함과 레거시를 명시하기
// e.g. TODO: JPEG외 다른 이미지 포맷도 처리할 수 있어야 한다.
- 상수에 대한 설명 
// e.g. 합리적인 한계 - 1000개 이상을 구독하는 사람은 기획 논의상 없다.
- 쉽게 빠질 수 있는 함정 미리 경고하기
// e.g. 이 함수는 외부 서비스를 호출해서 이메일을 발송한다. (1분정도 소요)

+ JSdoc 문법을 가져다 쓰면 거의 모든 주석 문제가 해결된다.
+ TypeScript를 쓰면 주석의 양이 줄어든다.

2. let 대신 const를, 전역 대신 지역변수

let으로 선언된 전역변수가 있다면 어디서 값이 바뀌었는지 추적하기 쉽지 않다. 특히나 웹프론트는 상태나 함수의 동작을 예측하기 너무 어려운 웹 프론트엔드에서는 특히나 중요한 사항이다.

+ 추천키워드: 순수함수

3.  함수명, 변수명을 의미 있게

testCase1(), testCase2() 와 같은 함수명 보다는 이름만 보고 어떤 테스트인지 알 수 있는 테스트 이름으로 고민해보자. inputs[0], inputs[1]은 이름이 아니다. const [count, name] = [inputs[0], inputs[1]]과 같은 식으로 의미있는 이름에 담아주자. 이런 경우에는 재사용이 안되더라도 변수에 담아 사용해도 좋다. 단, 값의 역할이 분명하고 한 번만 사용되는 변수라면 임시 변수 선언하는 것은 불필요하다.

4. 매개변수 개수는 2개 이하로

이상적인 매개변수의 갯수는 0~2개이다. 그 이유는 다음과 같다. 우선 2개만 되더라도 전달해야 할 인수의 순서를 고려해야 하는 부담이 생긴다. 또 매개변수의 개수가 증가할 때마다 테스트해야 할 경우의 수가 기하급수적으로 많아진다. 인자가 많다는 것은 또 하나의 메서드에 너무 많은 일을 하도록 하고 있다는 반증이기도 하다. 꼭 여러 변수를 보내고 싶다면 의미 있는 이름을 가진 객체에 담아 보내는 것도 방법이다. 예를 들어 startDate와 endDate를 보내고 싶다면 dateRange라는 객체에 담아 보낼 수 있다.

+ 추천키워드: arguments 객체, rest parameter (가변인자를 처리하는 방법)

5. 관리의 대상이 아닌 것은 push X

cypress 샘플파일, ./node_modules 등은 관리가 필요하지 않기 때문에 원격저장소에 푸시할 필요가 없다

6. 기타

class를 추가, 삭제, 확인하려면 className보다는 classList를 사용하자.
if문은 중괄호로 묶어주자.
switch문을 사용한다면 default를 지정하자.
분기 안에서 분기는 예외사항을 체크하기 어렵게 한다. depth를 낮추자.
10진수의 형변환이라면 + 나 parseInt 대신에 Number()를 사용하자
메서드의 단위를 설정할 때 테스트 가능한 코드와 테스트하기 힘든 부분을 분리하도록 한다.
라이브러리에서 가져오거나 기능이 뚜렷한 함수는 테스트할 필요가 없다.
PR을 보낼 때 구현한 요구사항을 체크하면 현재 진행한 상황과 피드백 줄 내용이 명확해진다.

 

 

자동차경주 미션 🏎

1. 디자인패턴을 위한 디자인패턴?

디자인패턴은 문제 해결을 위한 전략들이다. 내가 어떤 문제를 해결하는지 알고 사용하는 것이 중요하다. 

I often say that patterns are half-baked— meaning you always have to finish them yourself and adapt them to your own environment. - '리팩토링'의 저자 마틴 파울러

예를 들어 과거 정적으로만 존재하는 웹사이트에서는 MVC패턴과 같은 문제 해결 방법이 필요하지 않았다. 사용자와의 인터랙션이 늘어난 후에 MVC MVVP와 같은 패턴들이 등장하게 된 것이다. 어떤 배경에서 어떤 문제가 있어서 해당 패턴이 나타났는지를 고민하고 이해하는 것이 중요하다.

처음부터 이미 마련된 프레임을 가지고 있으면 프레임 이상의 것을 상상하기 어렵다. 패턴에 종속되어있으면 패턴을 위한 코드를 작성하기 쉽다. 자동차 경주 PR을 보면 꼭 이렇게까지 패턴을 사용해야 하나 싶은 경우도 있었다. 패턴에 끼워 맞추느라 생기는 레거시를 작성하지는 않았는지 고민해보자. 켄트 백 등이 주창한 익스트림 프로그래밍(XP)의 원칙, YAGNI (You Aren't Gonna Need It, 정말 필요하다고 간주할 때까지 기능을 추가하지 않는 것이 좋다.) 를 기억하자. 나중에 필요할 것 같아서 만들어 놓으면 안 쓰는 경우가 많다.

2. 프론트엔드 개발자라면 사용성 고민을!

어떤 패턴의 코드를 작성할지 보다 사용자와의 상호작용을 더 고민해보자. 프론트엔드 개발자라면 사용자의 입장에서 어떤 기능이 필요할지, 어떻게 기능을 만들어야 사용하는 입장에서 편리할지를 염두에 두고 개발해야 한다. 자동포커스 기능을 통해 사용자가 프로그램이 시작되면 가장 먼저 해야 할 일을 암시하거나 인풋에서 엔터와 클릭 모두 사용 가능하도록 하는 것은 아주 좋은 예시이다.

3. 피드백 단순수용을 넘어 한 단계 더

리뷰어가 알려준 내용을 찾아보고 적용하는 것도 좋지만 어떻게 더 효율적으로 사용할 수 있을지 페어와 고민해보고 더욱 발전시켜보자. 예를 들어 querySelector 스니펫을 리뷰어가 제안해준 이후 많은 크루들이 그대로 사용하고 있다. 그대로 사용해도 좋지만 어떻게하면 더 발전시킬 수 있을지 생각해보자.

const $ = (selector) => document.querySelector(Selector);
const $ = (selector) => document.querySelectorAll(Selector);

4. 도메인 로직에 포함할 것 구분하기

도메인 로직에 포함할 것과 포함하지 않을 것을 구분하는 것은 의식적으로 노력하지 않으면 틀리기 쉽다. 순수 함수를 객체 안에 넣고 싶은 경우, static 메서드로 선언해주면 인스턴스와 상관없이 사용할 수 있다. 참고로 ESLint에서도 class의 메소드에 this가 사용되지 않으면 warning이 뜬다.

5. 지연시킨다는 것은...

자동차 전진을 1초 동안 지연시키는 기능의 출제의도는 sleep과 같이 그저 코드 실행을 지연시키는 것이 아니라 서버로부터 비동기적으로 데이터를 받아 화면에 그려내듯 결과를 비동기적으로 얻어서 사용해보자는 것이었다. 자바스크립트에서 비동기는 미래에 줄 값을 약속하는 것이다. 이는 마치 카페에서 받는 진동벨과 같다. 주문을 하고 나면은 진동벨(약속, Promise)를 받는다. 벨이 울리면 주문한 것을 받을 수 있다. sleep을 하는 건 주문하기 전에 진동벨을 받아간 것이라고 할 수 있겠다. (참고 피드백) 다음 미션인 유튜브 강의실을 진행하다 보면 이 부분이 좀 더 명확해질 것이다.

+ 추천키워드: 명령형 코드, 선언형 코드.

6. 이벤트핸들러에 이름을 붙여주면

이벤트핸들러를 익명함수로 작성하는 경우가 종종 있었다. 이렇게 인라인으로 작성할 경우에 처음 해당 코드를 본 사람은 콜백함수 '전체'를 읽어야만 비로소 어떤 내용인지 파악할 수 있다. 반면, 'handleCarNameInput'와 같이 해당 함수가 어떤 '역할'을 하는지 드러낼 수 있도록 이벤트핸들러에 이름을 붙인다면 전체 코드를 읽지 않아도 바로 파악이 가능해진다.

'handle'과 같은 prefix를 사용하는 것은 좋지만 'handleRestartButtonClick' 과 같이 이벤트 행위를 명시할 필요는 없다. 클릭했을 때, 엔터키를 눌렀을 때 모두 지원하는 함수의 이름을 상상해보면 이유가 납득이 갈 것이다. 지원하는 이벤트가 늘어나더라도 함수 이름을 바꿔도 되지 않도록 함수가 하는 '역할'을 이름으로 주는 것이 베스트이다.

7. 기타

for문과 같은 제어문은 휴먼에러를 유발한다. Array 메서드를 잘 활용하자.

[...Array(arr.length)].map(() => getRandomNumber(0, 10))

 

 

로또 미션 🎟

1. 조건은 자동문~

아래와 같은 조건은 거의 자동문과 같다. 원시형이 아닌 자료형은 모두 객체이다. 이렇게 object인지 검사하는 조건문 작성은 지양하고 검증된 validate 라이브러리를 사용하자. (참고링크 ansman/validate.js , validatorjs/validator.js)

if (typeof winningNumber === 'object') {
  this.winningNumber = winnningNuber;
}

2. 만능함수 네이밍은 지양

hasDuplicate와 같이 object일지 array일지 가늠할 수 없는 네이밍 보다는 hasDuplicatedElement, hasDupilicatedValue 처럼 풀어서 쓰는 것이 좋다.

3. 매직넘버는 기호상수로

코드 내 하드코딩 되어있는 숫자 또는 문자열을 매직넘버(Magic number)라고 한다. 애매하게 매직넘버를 남겨놓지 않고 모든 숫자를 기호 상수(Symbolic Constant)로 다룰 수 있도록 처리하자.

4. 클래스 사용 유의사항

추상클래스는 각 클래스의 공통점만 뽑아서 작성해야 한다. 추상클래스를 extends 한 클래스에서 공통되지 않는 프로퍼티나 메서드가 있다면 추상클래스의 역할을 다시 생각해보자. 또, 한 계층 안에서 인스턴스를 어떤 함수에서는 인자로 받고, 어떤 함수에서는 직접 생성하는 것과 같이 일관되지 않게 인스턴스를 다루는 방법을 경계하자. 이런 복잡성은 협업에 방해가 될 수 있다. 

5. 새로운 방식을 시도했다면

새로운 방식을 시도해보는 것은 좋다. 단, 새로운 방식을 시도했다면 그 시도를 리뷰어에게 더 어필해보고 피드백을 요청해서 나의 코드가 어떤지 의견을 구해보자. DOM 조작 최적화에 대해 고민하고 실험해보자.

6. 기타

웹표준을 지키자. (중요)
id가 점차 많아지고 있다면 data-* 속성을 고려해보자.
NodeList는 Array는 아니지만 forEach()를 사용하려는 목적이라면 굳이 Array로 변환하지 않아도 된다.
JS DOC의 활용도 고려해보자.
불필요한 코드는 반드시 제거하고 PR을 올리자.
실수는 예방하는 것이 아니라 관리하는 것이다.
PR 파일 마지막 부분에 빨간표시가 뜬다면 EOL을 검색해보자. (참고)

+ 프론트엔드 크루 26명 모두가 틀린 문제...!

new 연산자가 없기 때문에 정답은 undefined...!

 

유튜브 강의실 미션 🧑🏻‍🏫

1. 산문이 아닌 시를 쓰자

프로그래밍 실력이 높은 사람이 작성한 코드일수록 각가의 알고리즘을 짧고 간결하다. ...(중략)... 그런 코드는 다른 논리를 위해 확장되기 쉽고, 코드 자체가 수행하는 일이 뚜렷하고 간단해서 유닛테스트를 수행하기가 용이하다. 코드가 수행하는 일이 뚜렷하고 간단해서 유닛테스트를 수행하기가 용이하다. ...(중략)... 날마다 수십 혹은 수백 줄의 코드를 작성하는 프로그래머라면 잠시 생각해보기를 바란다. 매일 작성하고 있는 자신의 프로그램이 산문인지 아니면 시인지를. 비론 현실은 건조한 산문을 쓰라고 강요할지라도 스스로 시를 쓰려고 노력하는 프로그래머의 자세는 그를 한층 높은 추상의 세계로 이끌게 될 것이다. 

- 마이크로소프트웨어 2006년 6월호

2. localStorage 사용시 주의사항

JSON.parse()의 인자가 유효한 JSON이 아닐 경우 SyntaxError가 발생한다. 반대로 JSON.stringify()에서 순환참조가 발생할 경우 TypeError가 발생한다. 그 외에 직렬화할 수 없는 경우(undefined, Symbol, function)에 대해서도 예외처리가 필요하다.

3. 클로저를 활용한 Throttle

클로저를 활용하면 전역에 timerId를 두지 않고도 클로저를 구현할 수 있다.

export const doThrottling = (func, ms) => {
  let timerId;

  return () => {
    if (!timerId) {
      timerId = setTimeout(() => {
        timerId = null;
        func();
      }, ms);
    }
  };
};

4. 삼항연산자와 '값식문'

삼항연산자는 값을 평가해서 반환한다. 아래와 같이 단순히 실행만을 위한 삼항연산자를 사용하는 것은 개인의 취향 영역일 수 있지만 사용 시 값이 반환된다는 점을 인지하고 사용하자. (관련 질문 및 리뷰어님 피드백)

isWatching ? (this.watchingVideoCount -= 1) : (this.watchedVideoCount -= 1);

 

 

지하철 노선도 미션 🚇

1. 라이브러리 활용은 좋지만

외부의 유용한 라이브러리를 활용하는 것은 좋다. 하지만 그대로 코드를 가져와 사용하기 보다는 기존의 애플리케이션 코드와 결이 맞게끔(var키워드 사용 지양 등) 구현하는 것이 좋겠다. 또는 아예 관리하지 않는 소스코드로서, npm과 같은 패키지 매니저를 통해 외부 라이브러리로써 관리하는 것이 좋겠다. 

/*
 *
 *  Secure Hash Algorithm (encrypt)
 *  http://www.webtoolkit.info/
 *
 *  Original code by Angel Marin, Paul Johnston.
 *  유저의 패스워드를 암호화 해주는 라이브러리.
 */

export function encrypt(s) {
  var chrsz = 8;

  var hexcase = 0;

  function safe_add(x, y) {
    var lsw = (x & 0xffff) + (y & 0xffff);

    var msw = (x >> 16) + (y >> 16) + (lsw >> 16);

    return (msw << 16) | (lsw & 0xffff);
  }

  function S(X, n) {
    return (X >>> n) | (X << (32 - n));
  }
  ...
}

2. 같은 키워드가 반복된다면

isValid라는 키워드가 계속 반복되고 있다. 이 역할을 하는 객체를 하나 만들고 메서드로 구현하면 더 명확한 계층분리를 이룰 수 있겠다. 또한, 함수로 추상화한 것들에 대한 시도는 좋지만 isValidPasswordConfirm는 사실 isEqualString에 가깝다. 이런 경우에는 유틸 함수를 추가하는 것도 좋겠다.

import { ELEMENT, STANDARD_NUMBER } from '../utils/constants';

export const isValidEmail = (email) => {
  const regExp = /^(([^<>()[\]\\.,;:\s@"]+(\.[^<>()[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/;

  return regExp.test(email);
};

export const isValidUserName = (userName) => {
  const regExp = /[^a-z가-힣]/i;

  return !regExp.test(userName);
};

export const isValidPassword = (password) => {
  return password.length >= STANDARD_NUMBER.PASSWORD_MIN_LENGTH;
};

export const isValidPasswordConfirm = (password, passwordConfirm) => {
  return password === passwordConfirm;
};