#045. 리팩터링: NULL + Boolean 대신 ENUM 타입 사용하기

문제

최근 한 API 작업에서 요청 받는 필드를 nullable한 boolean 타입으로 설정했다.
표시가 있으면(true) 표시된 모든 데이터를 가져오고, 표시가 되어 있지 않으면(false) 표시되지 않은 데이터만 가져온다. 그리고 아무 값도 주어지지 않으면(null) 모든 값을 다 조회하도록 하였다.

이 구현은 null 관점에서 아래 두 가지 원칙을 고려해봤을 때 개선할 부분이 있다.

(1) 함수로 받는 값에는 nullable을 최대한 피한다:
nullable을 허용하는 매서드는 프로그램 전체에 null 을 퍼뜨리는 역할을 한다. 매서드 파라미터에 null을 허용해 지나치게 유연한 메서드를 만들지 말고 최대한 명시적인 메서드/함수를 만들어야 한다. 

(2) boolean을 사용할 때 null은 배제하고 false 를 사용 하는 것이 좋다:
nullable 하게 만들어 null 과 false 를 구분해야한다면 true/false 두 가지 타입이 아닌 3가지 타입이 된다.

말하자면 아래와 같은 선택지가 되는 것이다.

적용

아래와 같은 코드가 있다고 가정한다. 

// request.ts - bad
export class Request {
  ...
  
  @ApiProperty({ required: false, type: Boolean })
  @IsOptional()  // <---- decorator로 request를 validation을 하고 있지만 nullable을 허용한다
  @ToBoolean()
  @IsBoolean()
  hasCourse?: boolean; // true, false, null 허용
}

 

// repository.ts - bad

  async findPost(
    hasCourse?: boolean,
  ): Promise<number[]> {
	...
    
    if (hasCourse === false) { // <---- false 인 경우
      where.course = null;
    }

    if (hasCourse === true) { // <--- true 인 경우
      where.course = { $ne: null };
    }

	// <------ null 인 경우 위 "분기를 하지 않는다" 라는 로직이 암묵적으로 사용 됨
    
    return await this.repository
      .find(where, {
        fields: ['id'],
      })
      .then((rows) => rows.map((row) => row.id));
  }

입력 받는 값을 nullable 한 boolean 이 아닌, enum 타입으로 받으면 문제를 해결할 수 있다. 아래와 같이 true, false 그리고 모든 케이스를 포함하는 all로 받는다면 아래와 같이 수정할 수 있다. all을 처리하는 케이스를 더 명시적으로 작성할 수 있다.

// HasCourseType.ts

@Enum('code')
export class HasCourseType extends EnumType<HasCourseType>() {
  static readonly TRUE = new HasCourseType('true');
  static readonly FALSE = new HasCourseType('false');
  static readonly ALL = new HasCourseType('all');

  private constructor(
    private readonly _code: string,
  ) {
    super();
  }

  static codes(): string[] {
    return PostSortType.values().map((value) => value.code);
  }

  get code(): string {
    return this._code;
  }
}

 

// request.ts - bad
export class Request {
  ...
  
  @ApiProperty({ required: true, type: HasCourseType.codes() })
  @ToEnum(HasCourseType)
  hasCourse: boolean; // null을 허용하지 않는다
}

 

// repository.ts

  async findPost(
    hasCourse: boolean,
  ): Promise<number[]> {
  	...
    
    if (hasCourse === HasCourse.FALSE) {
      where.course = null;
    }

    if (hasCourse === HasCourse.TRUE) {
      where.course = { $ne: null };
    }
    
    if (hasCourse === HasCourse.ALL) { // <------- all 인 경우 위의 두 경우를 모두 포함한다
      where.course = {
      	$or: [null, { $ne: null }], // <------- 두가지를 모두 넣거나 delete를 이용해 제거할 수 있다 => delete where.course
      };
    }

    return await this.repository
      .find(where, {
        fields: ['id'],
      })
      .then((rows) => rows.map((row) => row.id));
  }

 

Reference

- 기존 작업 방식의 문제점이 어떤 부분인지 분석할 때 아래 글을 참고했다.
https://jojoldu.tistory.com/721