Clean Code - 부울(Boolean) 매개변수는 절대 피하라!

부울 매개변수를 사용하는 경우

클린 코드를 읽다가 부울 인수를 절대로 사용하지 말라는 강력한 주장을 보게 되었습니다. 부끄럽게도 전날 프로젝트를 개발하면서 함수 인수로 boolean을 사용하게 수정했는데 말이죠..ㅎㅎ..

 

함수의 인수로 부울(Boolean)을 사용하게 되면 해당 인수가 참인지 거짓인지에 따라서 다른 동작을 한다는 말이 되는데, 그러면 가독성도 나쁘고, 책임도 모호해집니다! 그래서 두개의 함수로 분리하는 것이 좋은 것 같습니다.

 

적용 방안

제가 작성한 기존의 코드는 아래와 같습니다. (설명을 위해 조금 수정했습니다.)

 

관리자 API Controller

findAllWithNotActivated = async ( req: Request, res: Response ) => {
        const result:CommonResult = await CategoryService.findAll(true);
        this.jsonResponse( res, HttpStatusCode.OK, result );
};

일반 사용자 API Controller

findAllWithNotActivated = async ( req: Request, res: Response ) => {
        const result:CommonResult = await CategoryService.findAll(false);
        this.jsonResponse( res, HttpStatusCode.OK, result );
};

Service Layer

findAll = async (withNotActivated?: boolean) => {
	// ...생략        
};

권한이 관리자면 컨트롤러가 서비스단의 findAll(true)를 호출하고, 일반 사용자면 findAll(false)를 호출합니다.

 

그래서, withNotActivated가 true면 비활성화 레코드도 조회에 포함하고, withNotActivated가 false면 비활성화 레코드는 포함하지 않게 해서 Service단의 findAll 메서드의 중복을 막고자 했습니다!

 

하지만 단일 책임 원칙도 무너지고, 함수 호출 코드가 CategoryService.findAll(false); 라고 되어있는데, 그러면 findAll을 까봐야지만 매개변수의 의미를 알 수 있습니다.

 

 

따라서, 아래와 같이 그냥 함수를 2가지로 분리하는 것이 바람직한 것 같습니다.

 

 

관리자 API Controller

findAllWithNotActivated = async ( req: Request, res: Response ) => {
        const result:CommonResult = await CategoryService.findAllWithNotActivated();
        this.jsonResponse( res, HttpStatusCode.OK, result );
};

사용자 API Controller

findAllWithNotActivated = async ( req: Request, res: Response ) => {
        const result:CommonResult = await CategoryService.findAllOnlyActivated();
        this.jsonResponse( res, HttpStatusCode.OK, result );
};

Service Layer

findAllWithNotActivated = async () => {};
findAllOnlyActivated = async () => {};

 

 

감사합니다.

의견 있으시면 댓글 남겨주세요 !