LMS 인증 로직을 리팩토링하며 발견한 문제점과 해결방안
F-Lab : 상위 1% 개발자들의 멘토링
안녕하세요! F-Lab 프론트엔드 개발자 Eden입니다.
최근 F-Lab 멘티분들이 사용하시는 LMS(학습 관리 시스템)의 인증 로직을 리팩토링했습니다. 잠재적으로 개발 속도를 늦추는 레거시 코드를 개선하기 위함이었습니다.
기존 코드의 문제점과 리팩토링시 고려한 점, 리팩토링 결과를 소개합니다.
먼저 기존 코드의 구조를 간략히 보여드립니다.
// withPassedApplicationInfo.tsx
const withPassedApplicationInfo = <P extends object>(WrappedComponent: React.ComponentType<P>): React.FC<P> => {
const HOC = ({ ...rest }: P): ReactElement => {
//.. 중략
return (
<ApplicationContextHolder applicationId={applicationId ? String(applicationId) : undefined}>
<LmsTemplate>
<WrappedComponent {...(rest as P)} />
</LmsTemplate>
</ApplicationContextHolder>
);
};
return HOC;
};
interface ApplicationContextHolderProps {
applicationId: string | undefined;
children: ReactElement;
}
const ApplicationContextHolder: React.FC<ApplicationContextHolderProps> = ({ applicationId, children }): ReactElement => {
const done = useAssertApplicationContext(applicationId);
return <>{done && children}</>;
};
export default withPassedApplicationInfo;
위 소스코드는 LMS 의 인증 여부를 검사하는 최상단 컴포넌트입니다.
JSX 구문을 보시면 ApplicationContextHolder 라는 컴포넌트가 있습니다. 해당 컴포넌트의 내부에서는 useAssertApplicationContext 훅을 호출합니다.
useAssertApplicationContext 는 LMS 관련 인증 여부를 검사하여 boolean 값을 반환합니다.
따라서, 반환 값이 true라면 LMS 인증을 통과했다고 판단하여 LMS에서 필요한 모든 하위 컴포넌트(children)을 보여주는 방식입니다.
이 글에서는 ApplicationContextHolder 와 인증 여부를 검사하는 useAssertApplicationContext 를 개선하면서 어떤 문제들을 해결했는지 소개 드리려 합니다.
목차
문제: 하나의 함수가 너무 많은 역할을 하고 있다
해결방안: 함수의 시그니처를 명확하게 하자
문제: undefined가 널리 퍼지고 있다
해결방안: falsy한 값은 얼리 리턴한다
문제: 함수가 호출하는 쪽에서는 예측하지 못한 동작을 한다
해결방안: 호출하는 쪽에서 함수의 동작을 예측할 수 있도록 하자
문제: 하나의 함수가 너무 많은 역할을 하고 있다
한 함수가 많은 역할을 하면 함수의 시그니처만 봤을 때 해당 함수가 어떤 일을 하는지 파악하기 어렵습니다. 따라서, 내부 구현을 모두 파악해야만 하는 리소스가 발생할 수 있습니다.
또한, 오류가 발생했을때 디버깅이 어려워질 수도 있습니다.
// useAssertApplicationContext.tsx
export const useAssertApplicationContext = (applicationId?: string): boolean => {
useEffect(() => {
(async (): Promise<void> => {
if (activeMenus) {
// 사용자가 선택한 메뉴에 권한이 있는지 확인하는 로직
}
if (!isAuthorized) {
// 사용자의 로그인 여부를 확인하는 로직
}
if (isNotValidApplicationId() {
// LMS 에서 사용할 applicationId를 특정하는 로직 약 100여줄
}
})();
}, []);
};
LMS 인증 로직을 담은 위 코드는 하나의 익명 함수 안에서 3개 이상의 역할을 하고 있습니다.
그리고, 함수의 시그니처를 보고 예측한 함수의 동작과 실제 함수의 동작도 조금 다릅니다.
해결방안: 함수의 시그니처를 명확하게 하자
함수의 시그니처는 함수의 이름, 함수가 받아들이는 매개변수, 그리고 함수의 반환 값을 의미합니다.
함수의 시그니처만 보고도 해당 함수가 어떤 역할을 하는지 이해할 수 있어야 좋은 함수라고 생각합니다.
만약 함수의 시그니처가 명확하게 느껴지지 않는다면 해당 함수는 많은 역할을 하고 있을 수 있습니다.
따라서, 함수의 역할을 더 작게 분리해보았습니다.
// 사용자가 선택한 메뉴에 권한이 있는지 확인하는 컴포넌트
interface Props {
children: ReactElement;
}
const NavPermission: FC<Props> = ({ children }) => {
const { status } = useRecoilValue(application);
const { isAllowed, isSuccess } = useValidateNavPermission({ status });
// 중략...
return children;
};
export default NavPermission;
사용자가 선택한 메뉴에 권한이 있는지를 확인하는 useValidateNavPermission을 NavPermission 에서 호출하여 권한이 확인되면 children 을 그려줍니다.
또한, ApplicationContextHolder 외부에서 로그인 여부를 확인하도록 수정했습니다.
만약 로그인이 되어있지 않다면, LMS 입장은 당연히 불가능하므로 LMS 인증을 거치기 전에 먼저 확인해야 한다고 생각했기 때문입니다.
이외에도 LMS 입장시 사용할 applicationId 를 특정하는 훅도 별도로 분리하여 생성했습니다.
interface UseMyApplicationIdParams {
enabled: boolean;
}
interface Return {
selectedApplicationId: string | null | undefined;
}
export const useMyApplicationId = ({ enabled: isApplicationIdNeeded }: UseMyApplicationIdParams): Return => {
// 중략 ..
return { selectedApplicationId };
};
함수의 시그니처를 명확하게 함으로써 함수가 하나의 역할에만 집중하게 했습니다.
한 역할만 하는 함수들로 분리하면서 많은 역할을 수행하던 useAssertApplicationContext은 필요가 없어져서 파일을 제거하기도 했습니다.
문제: undefined가 널리 퍼지고 있다
어떤 변수가 undefined 혹은 null 값을 가질 수 있다면 해당 값이 falsy한지 확인하기 위한 함수가 필요할 것입니다.
falsy한 값이 컴포넌트에 넓게 퍼지면 퍼질수록 값을 넘겨받은 곳에서는 반복적으로 falsy한지를 확인해야 할 것입니다. 이렇게 되면 코드의 의도가 분명히 전달되지 못할 수 있습니다.
interface ApplicationContextHolderProps {
applicationId: string | undefined;
children: ReactElement;
}
const ApplicationContextHolder: React.FC<ApplicationContextHolderProps> = ({ applicationId, children }): ReactElement => {
const done = useAssertApplicationContext(applicationId);
return <>{done && children}</>;
};
기존 코드에서는 applicationId의 값이 undefined일때도 useAssertApplicationContext 훅에 그대로 전달되므로 useAssertApplicationContext 내부에서 undefined 여부를 확인해야 했습니다.
이 때문에 useAssertApplicationContext 가 많은 역할을 수행하게 되고, if문도 깊이 중첩되었습니다.
해결방안: falsy한 값은 얼리 리턴하자
도메인 규칙상 applicationId 는 필연적으로 undefined 혹은 null일 수 있습니다.
따라서, 얼리 리턴을 통해 falsy한 값을 먼저 처리해주어 falsy한 값이 최소한으로 퍼지게 했습니다.
interface ApplicationContextHolderProps {
applicationId: string | undefined;
children: ReactElement;
}
const ApplicationContextHolder: React.FC<ApplicationContextHolderProps> = ({ applicationId, children }): ReactElement => {
const { isLmsAvailable, isSuccess } = useIsLmsAvailable();
const { selectedApplicationId } = useMyApplicationId({ enabled: isLmsAvailable && !applicationId });
const lmsApplicationId = applicationId || selectedApplicationId;
if (lmsApplicationId === undefined) return <LoadingStatus text={'loading...'} />;
if (lmsApplicationId === null) return <RedirectCourseSelect />;
return (
<MyLms applicationId={lmsApplicationId}>
<NavPermission>{children}</NavPermission>
</MyLms>
);
};
위 코드의 JSX 구문에서 MyLms 컴포넌트에는 항상 string 타입의 applicationId 가 전달됩니다.
falsy한 값을 가질때는 얼리 리턴을 통해 먼저 처리되기 때문입니다.
이를 통해 컴포넌트 내부에서 falsy한 값인지를 확인하는 반복 작업 및 복잡한 if문을 제거할 수 있었습니다.
문제: 함수를 호출하는 쪽에서는 예측하지 못한 동작을 한다
함수가 부수효과를 일으킬 수 있는데 호출하는 쪽에서 이를 예측하지 못한다면 함수가 작성된 의도와 다르게 잘못 사용될 가능성이 큽니다.
따라서, 호출하는 쪽에서 함수의 부수효과를 예측할 수 있도록 명시하는 것은 아주 중요합니다.
기존의 커스텀 훅 useAssertApplicationContext 은 호출하는 쪽에서는 예측하기 어려운 동작을 수행하고 있었습니다.
// ApplicationContextHolder (useAssertApplicationContext를 호출)
const ApplicationContextHolder: React.FC<ApplicationContextHolderProps> = ({ applicationId, children }): ReactElement => {
const done = useAssertApplicationContext(applicationId);
return <>{done && children}</>;
};
// useAssertApplicationContext.tsx
export const useAssertApplicationContext = (applicationId?: string): boolean => {
useEffect(() => {
(async (): Promise<void> => {
if (activeMenus) {
// 사용자가 선택한 메뉴에 권한이 있는지 확인하는 로직
}
if (!isAuthorized) {
// 사용자의 로그인 여부를 확인하는 로직
// 로그인되어있지 않다면 로그인 페이지로 리다이렉트한다
}
if (isNotValidApplicationId() {
// LMS 에서 사용할 applicationId를 특정하는 로직 약 100여줄
}
})();
}, []);
};
ApplicationContextHolder에서 useAssertApplicationContext를 호출하면서 사용자가 로그인 되어있지 않다면 로그인 페이지로 리다이렉트시킬 것이라는 사실을 쉽게 예측하기 어렵습니다.
직접 내부 구현을 살펴봐야만 알 수 있습니다.
ApplicationContextHolder을 개선하는 과정에서도 같은 이슈를 마주쳤습니다.
const ApplicationContextHolder: React.FC<ApplicationContextHolderProps> = ({ applicationId, children }): ReactElement => {
const { isLmsAvailable, isSuccess } = useIsLmsAvailable();
const { selectedApplicationId } = useMyApplicationId({ enabled: isLmsAvailable && !applicationId });
const lmsApplicationId = applicationId || selectedApplicationId;
if (lmsApplicationId === undefined) return <LoadingStatus text={'loading...'} />;
if (lmsApplicationId === null) return <RedirectCourseSelect />;
return (
<MyLms applicationId={lmsApplicationId}>
<NavPermission>{children}</NavPermission>
</MyLms>
);
};
만약 isLmsAvailable 가 false 라면 어떻게 될까요?
호출하는 쪽에서는 예측하기가 어렵습니다. useIsLmsAvailable 훅의 내부 구현을 살펴봐야만 알 수 있습니다.
해결방안: 호출하는 쪽에서 함수의 동작을 예측할 수 있도록 하자
ApplicationContextHolder에서 함수를 호출하는 모양새만 보더라도 이 함수가 어떤 역할을 할지 예측할 수 있도록 코드를 개선하였습니다.
const ApplicationContextHolder: React.FC<ApplicationContextHolderProps> = ({ applicationId, children }): ReactElement => {
const { isLmsAvailable, isSuccess } = useIsLmsAvailable();
useRedirect(
'/my-page',
{
beforeRedirect: () => alert('신청하신 코스 정보가 없습니다. 마이페이지에서 다시 접속해주세요.'),
enabled: isSuccess && !isLmsAvailable,
},
[isSuccess],
);
const { selectedApplicationId } = useMyApplicationId({ enabled: isLmsAvailable && !applicationId });
const lmsApplicationId = applicationId || selectedApplicationId;
if (lmsApplicationId === undefined) return <LoadingStatus text={'loading...'} />;
if (lmsApplicationId === null) return <RedirectCourseSelect />;
return (
<MyLms applicationId={lmsApplicationId}>
<NavPermission>{children}</NavPermission>
</MyLms>
);
};
위와 같이 isLmsAvailable 가 false일때의 동작을 호출하는 쪽에서도 알 수 있도록 외부로 꺼내기도 했습니다.
물론 모든 상황에서 위 예시처럼 함수의 동작을 밖으로 꺼내는 것이 정답은 아닙니다.
상황에 따라서 내부 구현에 숨기는 것이 유리할 때가 있고, 외부에서 주입하는 형태가 유리할 때가 있다고 생각합니다.
함수를 호출하는 쪽에서 어떻게 예측할 수 있을지를 기준으로 판단해보는 것이 좋겠습니다.
저는 앞으로도 코드를 작성할 때 호출하는 쪽에서 함수가 이런 동작을 할 것이라는 걸 예측할 수 있을까? 를 생각해보려고 합니다.
마치며
사소한 고민과 결정이 모여서 위대한 소프트웨어를 만든다고 생각합니다.
분명 리팩토링한 코드에서도 개선할 부분이 또 있을 것입니다. 사소해 보일지라도 더 나은 코드를 만들기 위해서 치열하게 고민하는 개발자가 되고 싶습니다.
저는 F-Lab 멘토링을 직접 결제하여 듣고 있는 멘티이기도 합니다.
리팩토링하는 과정에서 멘토님께 코드 리뷰를 받으며 리팩토링할 포인트를 찾고, 더 나은 표현 방법을 고민해볼 수 있었습니다.
긴 글 읽어주셔서 감사합니다. 모든 개발자분을 응원합니다.
엔지니어들이 코드를 작성할 때 내리는 일상적인 결정은
그것만 보면 작고 때로는 보잘것없어 보일 수도 있지만,
좋은 소프트웨어인지 그렇지 않은지는 그 모든 작은 결정들이 모여서 이루어진다.
<좋은 코드, 나쁜 코드 p.5>
사수가 없어 성장하기 힘드신가요?
F-Lab에서 빅테크 기업 타이틀과 실력을 겸비한 멘토님들께 실력 향상을 위한 멘토링을 받을 수 있습니다.
개발 경험이 있는 취준생이거나 7년 이하 경력 개발자라면 충분히 멘토링을 받아 뛰어난 개발자로 성장하실 수 있습니다.
이 컨텐츠는 F-Lab의 고유 자산으로 상업적인 목적의 복사 및 배포를 금합니다.
