[DEVLOG] 우당탕탕 제발모발 v2 개발기 - 탈모 진단 API 수정하기 (1) 변수 작명, Optional, Enum 활용

🏝 0. 서론

 

졸업작품으로 탈모 진단 어플리케이션인 '제발모발'을 개발하고, '해당 플랫폼을 자신의 회사에 이전해 사용하고 싶다'라는 어느 교수님을 만나 AWS 생태계에서 사내 서버 컴퓨터로 이전을 진행했습니다.

 

이전을 진행하면서 이미지 저장 등의 기능은 AWS 생태계에 의존했었기 때문에 해당 의존을 끊어내기 위해서 이미지 업로드 및 다운로드 API를 만들 일이 있었습니다. 그러면서 백엔드 레거시 코드를 확인했는데, 생각보다 문제가 많아 리팩토링 & 개선을 하기로 마음먹었습니다. 

 

퀄리티와는 관계 없이 해당 백엔드 서버를 이전하고 애플리케이션을 제공하는 것이 계약의 전부지만, 한 번 본 이상 그냥 지나칠 수는 없었습니다. 그래서 앞으로 여러모로 문제가 많은 코드를, 알아보기 쉬운 코드 / 재사용성이 좋은 코드 / 유연한 코드 / 실수를 줄일 수 있는 코드로 변경하는 과정을 하나씩 기록해보려고 합니다.

 

졸업작품 개발 당시 애플리케이션을 전담으로 맡았고, 각자 맡은 역할에 집중하고 데드라인 지키기를 우선순위를 높게 두었습니다.

그러나 백엔드 파트의 코드리뷰를 자주 하지 않고 주된 피드백 없이, 기능에 대한 검증이 어느정도 되면 무심코 넘겨버렸던게 큰 산이 되어 돌아왔네요. 다시금 한 번 코드리뷰의 중요성을 깨닫습니다.

 

우선적으로 손 볼 곳은 '진단' 파트입니다. 기존에 설문 + AI M자 탈모 이미지를 제출하는 진단 기능만 있었으나, 열화상 이미지로 진단하는 기능이 추가될 것으로 보입니다. 이에 기존의 구조 혹은 냄새나는 코드를 제거하고, 새롭게 개편을 하려고 합니다.

 

 

---

 

🧐 1. 기존 API 살펴보기

 

AS-IS

 

해당 부분은 서비스 레이어에 있는 진단 시작 시 초기화 작업을 하는 코드입니다.

진단 도중 강제적으로 종료한 경우에는, 진단 완료된 경우가 아니기 때문에 해당 기록을 폐기하려는 목적으로 DB에도 active라는 attribute로 관리되었습니다.

 

따라서 해당 active 여부에 따라 기록 폐기 진행 + 새로운 진단 기록을 생성하고 진단 번호를 얻어오는 것이 이 초기화 작업을 하는 부분입니다.

 

언뜻 봐도 문제가 굉장히 많아보입니다.

1. 메소드 목적을 정확하게 파악하기 어려운 이름
2. findById를 통해 얻은 값은 Optional 이지만, Optional을 사용한 의미가 퇴색되는 코드
3. 기록 삭제 3가지를 전부 한 트랜잭션에 넣지 않음.
4. "createLog" 라는 메소드가 빌더 패턴 내부에 인라인으로 들어가 해당 서비스에서 새로운 기록을 생성하는지 알아보기 어려움. 등등.

 

또한 추가적인 개선 사항은 다음과 같습니다.

1. 더 많은 정보를 담은 response + ResponseEntity를 활용한 응답.
2. 요청 내부의 값이 유효하지 않는 경우에는, Bad Request를 보내야 함.
3. 기존에는 active를 기준으로 로그를 '삭제' 했으나, 이 active를 state로 바꿔 7가지의 상태로 표현. 삭제가 아닌 'SHUTDOWN'으로 변경
4. 각 서비스마다 특정 요건에 따른 response를 리턴하게 되는데, 응답에 대해 유연한 사용 + 재사용성이 높은 구조 만들기

 

---

 

🧐 2. 메서드 / 변수 이름 변경

 

disabledService는, 서비스 자체에 장애가 있다는 것인지, 장애가 있는 무언가를 처리하는 작업인지 명확하지 않습니다.

또한 해당 메서드는 non-active한 요소를 제거하는 것이 핵심 목표가 아니라, "진단 생성"하는 것이 최우선 작업이며 이 두 가지를 묶어서 진단 초기화라고 표현할 수 있습니다.

 

따라서 조금 더 명시적인 이름으로 변경했습니다.

disabledService → forInitDiagnosis

 

사실, 메소드 명은 대부분 '동사'의 형태로 기술하는 것이 일반적입니다.

다만, 컨트롤러와 바로 맞닿아 있는 서비스의 메소드에 대해서만 앞에 'for'을 붙인 네이밍을 진행하도록 했습니다.

그 이후, 메소드 명만 보고 이 함수가 어느 레이어에 위치한 메소드인지 한 번에 확인할 수 있게 되었습니다.

 

return diagnosisService.disabledService(request) 
→ return diagnosisService.forInitDiagnosis(requestForInitDiagnosis);

 

이 외에도 기타 변수에 대해서 해당 코드를 처음 보는 개발자도 쉽게 이해할 수 있도록 풀어서 작성하고, 카멜케이스를 이용해 작성했습니다.

 

---

 

🤔 3. Optional을 사용하는 목적은 NPE 방어

 

다시보는 냄새 코드

 

기존의 코드를 보면,

userNum = memberRepository.findById(disabled.getId()).get().getUserNum();

이 부분에는 많은 문제가 있습니다. 

 

1) disabled.getId()에서 NPE 발생 가능성 존재
2) findById를 통해 얻어온 값은, Optional<T>인데, get() 메소드를 통해서 얻어오게 되면 내부가 비어있는 경우 똑같이 NPE가 발생함. Optional을 사용한 이유가 없음.
3) 최종 getUserNum에서도 NPE가 발생할 가능성이 존재함.

물론 try-catch 문으로 막아서 프로그램이 중단될 상황은 모면하겠지만, 이 중 하나라도 NPE가 뜰 경우였다면, 사용자의 요청이 부실한 경우이므로 bad request를 보내는 것이 맞다고 판단했습니다.

 

기존의 코드를 수정한 결과는 다음과 같습니다.

 

 /* 불변 값 정의 */
final String REQUEST_HAVE_NULL = "REQUEST_HAVE_NULL";
final int NO_REQUEST_USER_IN_DB = -1;

/* requestBody 에서 userID 검출 */
String userIDFromRequest = Optional.ofNullable(requestForInitDiagnosis)
	.map(JBMBRequestObject::getRequestBody)
	.map(DiagnosisDTO.RequestForInitDiagnosis::getUserID)
	.orElse(REQUEST_HAVE_NULL);

/* BAD REQUEST => Null 값을 포함한 REQUEST */
if (userIDFromRequest.equals(REQUEST_HAVE_NULL)) {
	return 응답 생략;
}

/* READ => userID 기반으로, 멤버 식별 번호 검색 */
int userNumFromRequest = memberRepository.findById(userIDFromRequest)
	.map(Member::getUserNum)
	.orElse(NO_REQUEST_USER_IN_DB);

/* INTERNAL ERROR => DB에 해당 유저가 없음. 허용되지 않은 접근 처리 */
if (userNumFromRequest == NO_REQUEST_USER_IN_DB) {
	return 응답 생략;
}

 

문제가 많은 한 줄이었던 코드가, 꽤 늘어났습니다.

 

Optional은 내부의 값이 비어있는 상태라면, "orElse" 구문 시리즈로 해당 비어있는 값을 NPE를 발생시키지 않고 대체합니다.

public T orElse(T other)
public T orElseGet(Supplier<? extends T> other)
public <X extends Throwable> T orElseThrow(Supplier<? extends X> exceptionSupplier)

orElse와 orElseGet은 Optional 객체의 값이 Optional.empty()의 값인 경우 other의 값을 대체해서 보낼 수 있습니다.

만약 Optional 객체의 값이 비어있지 않다면, Optional<> 껍질이 벗겨지고 내부의 값 그대로를 가지게 됩니다.

 

orElse와 orElseGet의 다른 점은 크게 두 가지입니다. 

1. orElse의 인자값은 T이기 때문에, 해당 오브젝트를 그냥 넣어주면 됩니다. (혹은 해당 오브젝트를 리턴하는 함수)
orElseGet의 인자값은 Supplier입니다. 람다식을 이용해서 값을 넣어주어야 합니다.

2. 또 다른 점 하나는, orElse는 Optional 내부가 비어있던 아니던, 일단 실행되고 나중에 값이 결정됩니다.
orElseGet은 Optional 내부가 비어있다면, 내부의 값이 실행되지 않습니다.
만약 orElse의 인자에 T를 리턴하는 메소드지만, 내부에서 DB 조작등의 기능이 돌아간다면 의도와는 다르게 인자 내부가 실행되기 때문에 문제를 일으킬 수 있습니다. 이런 경우에는 orElseGet을 사용해야 합니다.

 

orElseThrow는 마찬가지로 Optional.empty()인 경우 내가 직접 Exception을 일으킬 수 있습니다. 커스텀한 Exception 혹은 에러 메시지를 직접 작성할 수 있다는 점에서 장점이 있습니다.

 

Optional 객체에 대해 .get() 메소드를 사용하면, 내부의 값이 비어있다면 NPE를 발생시키므로 Optional을 쓴 의미가 없습니다.

위와 같은 메서드를 사용하는 것이 권장되는 방법입니다.

 

 

이러한 Optional 객체에 대해 map 메서드로 내부의 값에 특정 메소드를 적용시킬 때 마다 자동으로 Optional이 씌워지게 됩니다.

예를 들어, 다음과 같은 클래스가 있다고 해봅시다.

@AllArgsConstructor
@Getter
class Example{
	private String test;
}

Example example = new Example("hi");

Optional.ofNullable(example)
	.map(Example::getTest) // 이때, Optional<String>이 됨.
    ... 생략

Example의 getter인 getTest는 String 타입을 리턴합니다. 그러나 map을 통해 호출이 되면, 자동으로 Optional이 씌워지게 된다는 것입니다. 이 점을 이용해 연속적인 NPE 위험에서도 벗어날 수 있습니다.

 

N중으로 if 문을 사용해 null 체크를 해야 하던 것이 없어지고 map을 사용함으로써 가독성을 높이고 연산 순서를 더욱 직관적으로 볼 수 있습니다.

 

기존에 try-catch 문에 모든 로직이 다 들어있었지만, try-catch를 제거하고 불변 값을 정의해 해당 값과 비교해 응답을 보내는 로직으로 변경했습니다.

 

try-catch 문 내부에서 선언한 지역변수는 밖에서 사용할 수 없어 미리 선언한 후 값을 변경해야 한다는 점을 피할 수 있으며, 어느 부분에서 return이 발생하게 되는지 개발자가 더욱 직관적으로 확인할 수 있다고 판단했습니다. 미리 Exception 타입을 모두 예측해 catch문을 여러개 잇는 것 보다 말이죠.

 

---

 

🤔 4. Enum을 활용해보자.

 

Enum을 활용해보자는 마음을 먹은 데에는 여러 이유가 있습니다.

1) resultCode에 int 값을 넣고 있는데, 백엔드 / 프론트엔드 모두 어떤 결과값인지 정확하게 파악하기 어렵다.
2) active(0 혹은 1)에서 → state(OPEN, SURVEY, IMAGE_UPLOAD, DIAGNOSE, DONE, ERROR, SHUTDOWN)의 여러 상태로 나눠야 하는데, 이를 숫자(분간하기 어려움) 혹은 문자(개발자의 실수 여지가 많음)로 표현하는데에는 한계가 있음.
3) 어느 곳에서는 0과 1로 표현, 어느 곳에서는 해당 0과 1이 나타내는 문자 코드 값을 같이 묶고 싶음 등.

 

java에서의 Enum은 class이기 때문에 더욱 많은 활용이 가능하다길래, 적극적으로 프로젝트 개선을 하면서 활용해보고자 공부를 했습니다.

 

내부에서 사용한 Enum의 예시 중, 진단 타입을 기본형 / 열화상형을 정의하는 코드입니다.

@Getter
@AllArgsConstructor
public enum JBMBDiagnosisTypeIdentifier {

    DEFAULT(false),
    THERMAL(true);

    private boolean isThermal;

    public static JBMBDiagnosisTypeIdentifier getJBMBDiagnosisTypeWhetherIsThermal(Boolean isThermal){
          return Optional.ofNullable(isThermal)
                .filter(bl -> bl)
                .map(bl -> JBMBDiagnosisTypeIdentifier.THERMAL)
                .orElse(JBMBDiagnosisTypeIdentifier.DEFAULT);
    }
}


/* requestBody 의 'isThermal' 을 사용해 diagnosisType 검출 */
/* 'request' 에 isThermal 값을 보내지 않으면, 기본 진단으로 설정. */
JBMBDiagnosisTypeIdentifier diagnosisTypeFromRequest = Optional.of(requestForInitDiagnosis)
	.map(JBMBRequestObject::getRequestBody)
	.map(DiagnosisDTO.RequestForInitDiagnosis::getIsThermal)
	.map(JBMBDiagnosisTypeIdentifier::getJBMBDiagnosisTypeWhetherIsThermal)
	.orElse(JBMBDiagnosisTypeIdentifier.DEFAULT);

 

'진단 유형 식별자'라는 이름을 가진 Enum을 생성했습니다.

JBMBDiagnosisTypeIdentifier.DEFAULT와 JBMBDiagnosisTypeIdentifier.THERMAL 두 가지 유형이 있습니다.

java의 enum은 클래스이기 때문에 내부에 필드와 메서드를 가질 수 있습니다.

 

DEFAULT 인스턴스는 false 값으로, THERMAL 인스턴스는 true 값을 생성자를 통해 생성했습니다.

또한 메서드로는 정적 팩토리 메소드를 생성해, 조건에 맞는 Enum 인스턴스를 boolean 타입의 인자값을 받아 얻을 수 있도록 만들었습니다.

 

서비스에서는 위의 코드 예시처럼 'diagnosisTypeFromRequest'를 받아온다면, .name()을 통해서 'THERMAL' 혹은 'DEFAULT' 값을 얻을 수 있습니다. 

 

/* ShutDown 대상을 표현한 Enum */
public enum ShutDownTarget {
	EXIST, NOT_EXIST, ERROR;

	public static ShutDownTarget getShutDownTargetByCount(long countOfShutDown) {
		return countOfShutDown > 0L ? ShutDownTarget.EXIST : (countOfShutDown == 0L ? ShutDownTarget.NOT_EXIST : ShutDownTarget.ERROR);
	}
}

@Getter
@AllArgsConstructor
public enum JBMBDiagnosisStateIdentifier {

    OPEN(1, "진단 시작", true),
    SURVEY(2, "설문조사 진행", true),
    UPLOAD(3, "이미지 업로드", true),
    DIAGNOSE(4, "진단 중", true),
    DONE(5, "진단 완료",false),
    ERROR(6, "에러 발생", false),
    SHUTDOWN(7, "진단 중단", false);

    private int stateCode;
    private String stateDescription;
    private boolean interruptible;

    public static boolean isShutDownAble(JBMBDiagnosisStateIdentifier nowState) {
        return nowState.isInterruptible();
    }

}
    

/* CommonDiagnosisService.java */
@Transactional
public long updateStateToShutDown(String requestUserID, int requestUserNum) {
	// UPDATE => SHUTDOWN 으로 상태 변경 및, 변경 대상 수 리턴
	try {
	Stream<String> shutDownedDiagnosisInfoIDList = diagnosisInfoRepository.findAllByUserNum(requestUserNum)
		.stream()
		/* SHUTDOWN 으로 상태를 변경해야 할 대상만 필터링 */
		.filter(diagnosisInfo -> JBMBDiagnosisStateIdentifier.isShutDownAble(diagnosisInfo.getDiagnosisState()))
		/* state SHUTDOWN 으로 변경 후 저장 */
		.map(JBMBDiagnosisInfoEntity::setDiagnosisStateToShutDown)
		.map(diagnosisInfoRepository::save)
		/* 변경 대상 진단 아이디 목록으로 변경 */
		.map(JBMBDiagnosisInfoEntity::getDiagnosisID)
		.map(Object::toString);

	log.info("=> Member [{}]'s diagnosis log is shut down, count : {} / changed ID : [{}]",
		requestUserID,
		shutDownedDiagnosisInfoIDList.count(),
		shutDownedDiagnosisInfoIDList.collect(Collectors.joining(", ")));

	return shutDownedDiagnosisInfoIDList.count();
	} catch (Exception e) {
		log.error(e.getMessage());
		return -1L;
	}
}

/* UPDATE => SHUTDOWN 으로 상태 변경 및, 변경 대상 수 리턴 */
/* SHUTDOWN 변경과 같은 경우, 실패해도 크게 타격이 없음. 다음에 다시 시도해도 됨. */
ShutDownTarget shutDownTarget = ShutDownTarget.getShutDownTargetByCount(
	commonDiagnosisService.updateStateToShutDown(userIDFromRequest, userNumFromRequest));

 

 

7가지의 진단 상태를 Enum으로 표현한 것과, SHUTDOWN 대상 존재 여부를 Enum으로 표현한 것의 예시입니다.

Enum을 사용함으로써 이점은 많았습니다.

1) 동시에 여러 값을 포함할 수 있었습니다. 이해할 수 없던 숫자코드만 덜렁 있던 이전과는 다르게, 해당 숫자코드가 나타내는 메시지, 이름, 등등을 같이 한 객체 내에 표현할 수 있었습니다.

2) 단순한 숫자, 문자의 표현이 아닌 Enum 타입의 객체를 사용하기 때문에 개발자의 이해가 더 쉽고, 직관적입니다. 또한 String 문자를 잘 못 입력하는 사고를 피할 수 있습니다.

3) 새로운 진단 상태가 생긴다하더라도, Enum의 생성자 부분만 만진다면 다른 로직의 수정 없이 유연한 확장이 가능합니다.

 

 

다음 일지에서는 DTO와 관련된 내용과, 공통 응답에 대한 처리, 재활용성이 높은 ResponseCreator를 만든 내용에 대해 서술하도록 하겠습니다.

[DEVLOG] 우당탕탕 제발모발 v2 개발기 - 탈모 진단 API 수정하기 (2) 공통필드가 많은 DTO 만들기