[node] 코드리뷰를 통한 코드의 발전(falsy)

2020-04-28일에 작성한 http 모듈과 express에서 헤더과 관련된 글이다.

코드리뷰와 1차 피드백

외부에서 넘겨 받은 헤더를 이용한 request 객체에 프로퍼티 추가에 대한 작업은 아래와 같은 형태였다.
Java를 주로 해서 그런지 타입에 대한 엄격한(strict)검사의 코드로 구성했다.

const uuid = require('node-uuid');

const injectTransactionId = (req, res, next) => {
  const xTraceId = req.header('X-Trace-Id');
  if (typeof xTraceId === 'string' && xTraceId.length > 0) {
    req.id = xTraceId;
    next();
  } else {
    req.id = uuid.v4();
    next();
  }
};

위의 코드로 PR(pull request)를 올렸더니 peer였던 임*혁님이 아래와 같은 피드백을 주셨다.

1. req.header('X-Trace-Id') 결과에 대해 empty check만 해도 무방할 듯 보입니다.
2. next 호출을 분기 종료 다음으로 이동하면 코드중복을 제거할 수 있을 것 같습니다.

일단 2번에 대해서는 바로 수용(accept)을 했다.
왜냐하면 PR을 만들고 나서 다시 퇴코(퇴고)를 하는데 내 눈에도 extract 할 대상으로 생각했기 때문이다.

1차 피드백의 의문점

그런데 결과에 대한 빈 값 체크에 대해서는 의문점이 들었다.
원래 PR 코드에서 빈문자열에 대한 체크는 바로 아래 코드가 수행하고 있었기 때문이다.

xTraceId.length > 0

따라서 comment에 대한 의견은 결과에 대한 타입 체크를 빼는 것인가 생각했고 express의 request 객체의 header의 동작에 대해 다시 한번 살펴보았다.
그 결과, request.header 는 아래와 같이 정의되어 있었고 결국은 this.headers에서 lower case인 키의 값을 돌려주는 것이었다.

req.get =
req.header = function header(name) {
  if (!name) {
    throw new TypeError('name argument is required to req.get');
  }

  if (typeof name !== 'string') {
    throw new TypeError('name must be a string to req.get');
  }

  var lc = name.toLowerCase();

  switch (lc) {
    case 'referer':
    case 'referrer':
      return this.headers.referrer
        || this.headers.referer;
    default:
      return this.headers[lc];
  }
};

this의 headers는 native node의 http 모듈의 message.headers 였고 그냥 Object 타입이다.
따라서 해당 키에 대한 값이 없다면 undefined를 돌려주게 된다.

1차 피드백의 피드백

따라서 타입 체크를 하지 않으면 empty check  역시 할 수 없기¹에 피드백에 대한 피드백을 달았다.

req.header()에서 값이 없으면 undefined 가 반환됩니다.
따라서 반환 타입이 String의 경우에만 empty check 를 수행하도록 하였습니다.
req.header: https://github.com/expressjs/express/blob/master/lib/request.js#L82
message.headers: https://nodejs.org/api/http.html#http_message_headers

2차 피드백

나중에 내 피드백에 대한 피드백이 또 달리고 나서야 empty check라고 했던 것이 falsy check 였음을 알게되었다.

검증식으로 아래와 같이하면 좀 더 간결해질 것 같네요.
if (xTraceId) {
req.id = xTraceId;
}

위와 같이 검증하면 null, undefined, empty string, false, 0, NaN 모두 false로 간주합니다.
https://developer.mozilla.org/ko/docs/Web/JavaScript/Reference/Global_Objects/Boolean

변증법

정(正)과 반(反)이 만나 합(合)이 되듯이 코드 리뷰에 대한 결과는 결국 단락배정 관용구(Short Circuit Assignment idiom)로 귀결이 되었다.

아래와 같이 if 문과 falsy를 이용한 방법보다는 

const xTraceId = req.header('X-Trace-Id');
if (xTraceId) {
req.id = xTraceId;
} else {
req.id = uuid.v4();
}

아래와 같이 logical OR를 이용한 관용구를 이용하는 것이 더 깔끔하다.

const xTraceId = req.header('X-Trace-Id');
req.id = xTraceId || uuid.v4();
next();

Thanks to

사실 이 코드 리뷰에는 숨겨진 제 3의 인물이 있는데, 김*오 님께 단락배정 관용구에 대한 팁을 제안해주어 감사의 말을 전합니다.

 

---

1. TypeError: Cannot read property 'length' of undefined 가 발생한다.