리팩토링 이야기

요즘은 회사에서 react-admin 라이브러리를 사용한 백오피스 작업을 주로 담당하는데 고객들에게 발행한 인보이스를 관리하는 페이지가 하나 있다. 종종 인보이스를 직접 처리할 경우가 있어서 해당 인보이스를 미수, 취소 처리하는 기능을 예전에 추가했었다. 그 땐 직접 처리하는 건 이거면 될 줄 알았는데 최근에 수동으로 입금 완료 처리도 해야하는 경우가 생겼다. 그래서 일단 추가 기능도 작업은 했는데 간단했지만 내심 찝찝한 부분이 있었다. 피곤해서(오후 6시가 넘은 시각이었다...) 차마 더 고민을 못하고 PR을 올렸는데 아래와 같은 코멘트를 받았다. 

정말 배울 점 많은 CTO님..

내가 두루뭉실하게 찝찝했던 부분이 다시 떠오르며 창피했다. 내가 보완할 부분은

1. 비즈니스 로직과 ui가 커플링 되어있으니 분리하기

2. 처리 타입이 늘어날 때 마다 타입과 컴포넌트를 모두 건드리지 않도록 개선하기

라고 생각했다. 

1번은 코멘트로 지적 받은 부분이다. 2번은 내가 작업하면서 아 이건 아닌데.. 하면서 일단 넘어갔던 부분이었기 때문에 같이 개선해야 겠다고 생각했다. 

 

인보이스를 직접 처리하는 화면의 흐름은 아래와 같다. 

 

미수, 취소, 입금 완료 중 원하는 처리 버튼을 클릭 -> 한 번 더 확인하는 다이얼로그가 보임 (돈 관련이라 재차 확인 하도록 했다) -> 확인을 누르면 api 호출하여 관련 처리를 마침

 

아주 간단하다. 다만 미수, 취소 처리에는 다이얼로그에 사유를 선택적으로 입력할 수 있는 텍스트 필드가 필요하고 그 외 처리에는 사유 입력이 필요없다. 그래서 처리 타입에 따라 어떤 건 보이고 어떤 건 안 보이고 엔드포인트와 바디가 다르고.. 이런 부분들을 다 분기 처리를 하다보니 ui와 로직이 뒤섞일 수 밖에. 처음부터 머리에 그려지지 않을 땐 일단 만들고 개선하는 편이기도 하지만.. 이번 작업도 그랬어야 했는데..!!

 

먼저 간단한 2. 처리 타입이 늘어날 때 마다 타입과 컴포넌트를 모두 건드리지 않도록 개선하기 부터 개선해본다. 

기존 코드는 대략 아래처럼 되어있었다. 동작은 잘 되지만 인보이스 처리 타입이 또 추가 된다면 미래의 내가 과거의 나를 많이 원망할 것 같은 코드다. 

// 기존 코드
const MyDialog = ({actionType}) => {

	const actionName = 타입에 따른 한글 이름 받는 함수(다이얼로그에 '미수 처리를 진행합니다' 처럼 보여줌)
    const url = 타입에 따른 엔드포인드 받는 함수
    const dataBody = 타입에 따라 api 요청할 때 사용할 바디 받는 함수
    
    ...
    
    return (
       ...
       {
         actionType === '입금 완료' ? null : (<텍스트 필드 />)
       }
    )
}

개선해보자. 타입은 추가 될 수도 있으니 타입은 수정하되 관련된 ui는 최대한 수정하지 않고 싶다.

// ActionTypes.js
const actionTypes = {
  needCancel: {
    label: "취소",
    endpoint: "취소처리 엔드포인트",
    showMemo: true,
  },
  needUncollectible: {
    label: "미수",
    endpoint: "미수처리 엔드포인트",
    showMemo: true,
  },
  needManualPaid: {
    label: "입금 완료",
    endpoint: "입금 완료 처리 엔드포인트",
    showMemo: false,
  },
};

// 다이얼로그 컴포넌트
const MyDialog = ({actionType}) => {
	const currentActionType = actionTypes[actionType];
    const {url, bodyData} = 엔드포인트와 바디를 리턴하는 함수(currentActionType)
    ...
    
    return (
       ...
       <p>{currentActionType.label} 처리를 진행합니다. </p> 
       {
         currentActionType.showMemo ? (<텍스트 필드 />) : null
       }
    )
}

이렇게 바꾸면 나중에 인보이스 처리 정책이 또 생겨나도 컴포넌트가 같이 수정될 가능성은 낮을 것 같다.

 

 

그럼 이제 1. 비즈니스 로직과 ui가 커플링 되어있으니 분리하기 부분을 개선해보자.

비즈니스 로직인지 판단할 때, 1) 리액트, 뷰 등 누가 와도 상관없이 공통적으로 적용할 수 있고 2) 이 프로젝트를 소유한 단체에서 정한 규칙에 따른는지 생각해보면 대부분은 쉽게 판단할 수 있다. 로직을 분리할 때 (리액트라면) 커스텀 훅을 사용하거나, 훅이 아닌 별도의 함수로 분리하는 방법을 주로 사용하게 되는 것 같다. 그런 다음 컴포넌트에서 필요한 곳에 호출해서 사용한다. (의존성 주입 - 나는 처음에 이 말이 정말 어려웠는데 그냥 필요한 기능을 호출하는 것이다 - 쯤으로 이해해도 충분하다고 생각한다.)

 

기존의 코드는 다이얼로그 컴포넌트 안에서 처리 타입에 따른 한글 이름, 엔드포인트 등을 얻고 있고 api 요청도 컴포넌트 내에서 모두 처리되고 있었다. 이들을 별도의 함수로 분리하고 컴포넌트에서 호출하는 식으로 분리한다. 

// invoiceService.js

export const constructApiUrlAndBody = (type) => {
  ... 처리 타입에 따른 엔드포인트와 바디 데이터 리턴하기
  return {url, body}
};

export const handleApiRequest = (type, invoiceId, memo) => {
  ... api 요청함
};

// 다이얼로그 컴포넌트
const MyDialog = ({actionType}) => {
	const currentActionType = actionTypes[actionType];
    // const {url, bodyData} = 엔드포인트와 바디를 리턴하는 함수(currentActionType)
    const {url, bodyData} = constructApiUrlAndBody(currentActionType)
    
    
    const handleClickConfirm = () => {
      handleApiRequest.then()
    }
    ....
    
    return (
       ...
       <p>{currentActionType.label} 처리를 진행합니다. </p> 
       {
         currentActionType.showMemo ? (<텍스트 필드 />) : null
       }
    )
}

 

이렇게 하고 다시 한 번 코드 리뷰를 요청해보아야겠다. 결과는 커밍쑨!