리팩토링 실습하기
 게시 됨 업데이트 됨etc19분안에 읽기 (약 2779 단어)

리팩토링 실습하기

약 한 달 조금 넘는 기간 동안 팀에서 듀데이트가 걸린 프로젝트를 완성하느라 블로그와 기타 공부를 완전히 손 놓고 정말 바쁘게 지냈다. 덕분에 머리도 2달 만에 자른 것 같고, 운동도 단 한 번도 안 했다. 책은 꾸역꾸역 받아서 읽긴 했는데 이전에 여유 있던 때처럼 읽지는 못 했다. 아무튼 현재는 그런 시간이 끝났고, 돌아가는 프로젝트의 코드베이스를 뭇지게 정리해두고 싶어져서 당분간은 기능 개발과 기존 기능들에 대한 리팩토링을 동시에 반반 정도 가져가려고 한다. 물론 새로운 기능들 역시 리팩토링을 진행 하면서! 이 글은 리팩토링 책을 읽은 후 우리 프로젝트의 코드베이스를 대상으로 실습을 진행하기 위해 어떤 리팩토링을 하게 되는지 정리한 글이다.

지난 번에 리팩토링을 읽고 간단한 후기를 작성한 적이 있는데, 해당 책을 읽고 난 후, 팀에서 짠 코드를 정리하는 것으로써 좋은 실습을 경험할 수 있을 것 같다고 생각이 들어서 팀원들과 함께 리팩터링 전과 후를 비교하는 세션? 스터디?를 진행하기로 했다. 리팩토링 책에서 말해주는 리팩토링 기법은 사실 우리가 이름 붙이지 않고, 자주 해오던 방식인 경우가 많다. 다만 해당 책에서 이름을 잘 붙여주고, 절차에 대해서도 잘 안내해주고 있기 때문에 이름과 절차를 차용해서 글을 작성했다.

앞으로 몇 번에 걸쳐서 적용할 예정이지만, 현재는 간단하지만 너무 간단하진 않은 내용들 위주로 리팩터링을 해볼 예정이다.

이번에 읽고 진행하는 부분은 다음과 같다.

  • 함수 추출하기
  • 함수 인라인하기
  • 함수 선언 바꾸기
  • 변수 선언 바꾸기

함수 선언, 변수 선언은 이름만으로도 적당한 방식으로 선언을 변경한다, 이름을 변경한다라는 것을 알기 쉬우니, 특별히 변경된 과정을 서술하지는 않을 것이다. 함수 추출하기는 React로 작성된 코드에서 유사한 컴포넌트에서 Base가 되는 부분들을 추출해 새로운 컴포넌트를 작성하는데 사용될 것으로 보인다. 함수 인라인은 개발과정에서 불필요하게 레이어 하나를 더 사용하고 있는 로직을 한 단계 플랫하게 만드는 과정을 진행할 것 같다.

함수 추출하기

우선 함수 추출을 하기 위해서는 아래와 같은 절차를 따라 리팩토링을 진행하라고 권고한다.

절차

  1. 함수를 새로 만들고 목적을 잘 드러내는 이름을 붙임 (무엇을 하는 함수인지 잘 드러나게 네이밍 해야 한다.)
  2. 원본 함수에서 추출할 내용을 복사 붙여넣기 한다.
  3. 추출된 코드 중 참조 하지 못하고 있는 변수를 찾아 매개변수화 한다.
  4. 원본 함수에서 추출된 함수를 호출하도록 변경한다.
  5. 테스트한다.

적용

클라이언트 부분에 복사 붙여넣기 코드의 산물로 다른 이름과 다른 상황에서 쓰이기는 하지만 UI가 동일하여 토시 하나 틀리지 않고 똑같은 코드가 몇 가지가 있다. 필자는 이러한 코드를 병적으로 싫어한다. 왜 컴포넌트를 재활용하지 않고 복사 붙여넣기 해야 하는가? 바쁘기 때문에 적절하게 수정하기 어려웠을 수 있지만, 프로젝트가 배포된 이후기 때문에 리팩토링에 시간을 들이며 현재 프로젝트에 그러한 문제가 있는 부분들을 수정해봤다.

책에서 마틴파울러는 “3 스트라이크”를 기준으로 리팩토링을 하라고 한다. 중복되는 경우가 두 번째 발생 하더라도 참고 넘기자는 건데, 충분히 재활용 될 법 하다고 판단되는 것들은 그냥 두 번 반복이 되어도 리팩토링 했다.

리액트는 함수를 추출한다는 의미가 컴포넌트화한다는 것과 유사한 느낌이 있다.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
// src/screens/QnAFileScreen/Presenter.tsx
import React from 'react';
import AuthImageComponent from '@/components/AuthImageComponent';
import DeleteHeaderContainer from '@/containers/DeleteHeaderContainer';
import styled from 'styled-components/native';

interface Props {
  token: string;
  uri: string;
  activateDelete: boolean;
  onPressDelete: () => void;
}

const Presenter = (props: Props) => {
  const { token, uri, onPressDelete, activateDelete } = props;
  return (
    <>
      <DeleteHeaderContainer title="첨부 파일" onPressDelete={onPressDelete} />
      <Container>
        <AuthImageComponent token={token} uri={uri} />
      </Container>
    </>
  );
};

export default Presenter;

const Container = styled.View`
  width: 100%;
  flex: 1;
`;
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
// src/screens/LicenseFileScreen

import React from 'react';
import AuthImageComponent from '@/components/AuthImageComponent';
import DeleteHeaderContainer from '@/containers/DeleteHeaderContainer';
import styled from 'styled-components/native';

interface Props {
  activateDelete: boolean;
  token: string;
  uri: string;
  onPressDelete: () => void;
}

const Presenter = (props: Props) => {
  const { token, uri, onPressDelete, activateDelete } = props;
  return (
    <>
      <DeleteHeaderContainer title="첨부 파일" onPressDelete={onPressDelete} />
      <Container>
        <AuthImageComponent token={token} uri={uri} />
      </Container>
    </>
  );
};

export default Presenter;

const Container = styled.View`
  width: 100%;
  flex: 1;
`;

위 두 가지 놀랍도록 단 한 자도 틀리지 않고 파일 내 모든 코드가 같은 두 예시는 다른 파일에 해당한다. 각각 QnAFileScreen, LicenseFileScreen에 해당하는데, 해당 스크린이 하는 역할은 첨부된 이미지 디테일을 보여주는 스크린이다.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
// src/containers/ImageDetailContainer/ImageDetailContainer.tsx

import React from 'react';
import AuthImageComponent from '@/components/AuthImageComponent';
import DeleteHeaderContainer from '@/containers/DeleteHeaderContainer';
import styled from 'styled-components/native';

export interface ImageDetailContainerProps {
  activateDelete: boolean;
  headerTitle?: string;
  token: string;
  uri: string;
  onPressDelete: () => void;
}

const ImageDetailContainer = (props: ImageDetailContainerProps) => {
  const {
    token,
    uri,
    onPressDelete,
    activateDelete,
    headerTitle = '첨부 파일'
  } = props;
  return (
    <>
      <DeleteHeaderContainer
        title={headerTitle}
        activateDelete={activateDelete}
        onPressDelete={onPressDelete}
      />
      <Container>
        <AuthImageComponent token={token} uri={uri} />
      </Container>
    </>
  );
};

export default ImageDetailContainer;

const Container = styled.View`
  width: 100%;
  flex: 1;
`;

위와 같이 내부를 옮겨 넣고, 다른 곳에서의 재사용을 위해서 적절하게 Props를 받아주도록 했다. 이제 이 부분을 위 두 가지 스크린의 Presenter 부분 대신 사용하면 된다. 두 스크린을 사용하는 곳을 ImageFileScreen과 같이 하나로 사용하고 싶은 생각도 들었지만, 묶여있는 Screen 부분의 Container 로직의 차이가 있고 Navigator와 엮여있는 부분도 있어서 이 정도까지만 했다.

함수 인라인하기

함수 인라인하기는 추출하기의 반대이다. 잘못 추출되었거나, 불필요한 간접 호출 등을 인라인 하면 된다. 아래와 같은 절차를 책에서 소개한다.

절차

  1. 다형 메서드인지 확인 (서브클래스에서 오버라이딩 하고 있다면, 인라인 하지 말 것)
  2. 인라인할 함수를 호출하는 곳들 모두 확인
  3. 각 호출문을 함수 본문으로 교체 (교체 할 때마다 해당 함수를 테스트한다.)
  4. 함수(인라인 된) 정의된 부분을 삭제한다.

적용

개발 중인 서버에서 결제하는 부분은 API 호출을 통해 이루어지는데, 해당 API 호출 코드와 데이터베이스 작업을 함께 처리하는 유틸 함수가 있었다. 개발 과정에서 API 호출과 데이터베이스 작업을 분리하게 되면서 API 호출하는 부분은 불필요한 레이어를 가지고 있게 되었다.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
@Injectable()
export class PaymentUtil {
  ... 
  private async payUsingCard(
    payment: Payment,
    prescription: Prescription
  ): ServiceData<PayApi.PayResponse> {
    try {
      ...
      
      // 실제 API 결제 처리 하는 곳

      return {
        success: true,
        result: payResponse
      };
    } catch (e) {
      return {
        success: false,
        error: e
      };
    }
  }

  // 실제 호출하는 메서드
  async pay(payment: Payment, prescription: Prescription): ServiceData<PayApi.PayResponse> {
    try {
      const pay = await this.payUsingCard(payment, prescription);
      if (!pay.success) throw pay.error;

      // 원래 데이터베이스 작업이 있던 자리

      return {
        success: true,
        result: pay.result
      };
    } catch (e) {
      return {
        success: false,
        error: e
      };
    }
  }
}

위 상황처럼 실제 호출하는 메서드는 단순히 payUsingCard라는 private 메서드를 간접 호출하고 있는 구조이다. 이 부분을 간단하게 표현하자면 아래와 같은 로직이라는 뜻이다. 나머지 에러처리를 위한 try, catch 부분은 데이터베이스 작업이 포함되어있던 잔재이다.

1
2
3
4
5
6
7
@Injectable()
export class PaymentUtil {
  async pay(payment: Payment, prescription: Prescription): ServiceData<PayApi.PayResponse> {
    const ret = await this.payUsingCard(payment, prescription);
    return ret;
  }
}

인라인 해야 하는 부분은 payUsingCard 메서드이고, 해당 메서드는 private이기 때문에, public으로 변경한 후 이름을 pay로 바꿔주었다. 기존의 pay 메서드는 제거했다.

위와 같은 비슷한 문제가 클라이언트 부분에도 있었다. React Native로 현재 작성된 코드는 Container Presenter 아키텍처를 갖추고 있다. 이 과정에서 불필요하게 컴포넌트를 구성해서 한 줄 짜리 구분을 나누는 등 불필요하게 Props를 내리거나, 또는 컴포넌트를 과하게 분리한 경우가 발생되었다. 아래와 같은 예시가 있다.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
import React from 'react';
import styled from 'styled-components/native';
import { Prescription } from 'bdyg-dto';

interface Props {
  historyDataList: Array<Prescription>;
}

const HistoryHeaderContainer = (props: Props) => {
  const { historyDataList } = props;
  return (
    <Wrapper>
      <ListCountText>{`전체보기(${historyDataList?.length}건)`}</ListCountText>
    </Wrapper>
  );
};

export default HistoryHeaderContainer;

const Wrapper = styled.View`
  flex-flow: row;
  padding: 0 5%;
  align-items: center;
  justify-content: space-between;
`;

const ListCountText = styled.Text`
  font-size: 14px;
  font-weight: 600;
`;

위 Header가 굳이 분리될만큼 복잡한 컴포넌트가 되어야 할까? 해당 부분은 컴포넌트간의 결합이나, 분리되어야 하는 로직이 존재하지 않는다. 단순하기 Wrapper와 Text만 가지고 있는 컨테이너이다.

팀에서 로직과 뷰를 분리하는 Container - Presenter 구조를 사용하면서도, 컴포넌트, 컨테이너, 스크린으로 구분하는 조합 단위도 존재한다. 이때 컨테이너라는 단어가 겹치기 때문에 팀 내부적으로는 로직 처리 하는 Container는 후자에서 말한 부분으로 이름 붙이고 (즉, 만약 HomeScreen의 로직을 담는 Container는 그냥 HomeScreen이 된다. HeaderContainer의 로직을 담는 Container는 HeaderContainer가 된다.) Presenter는 해당 디렉토리 안에 Presenter로 이름 붙이고 작성한다. 코드를 보면서 헷갈릴 것 같아 주석을 담았다.

이 부분이 굳이 Screen에서 분리되어야 할까? 불필요한 분리인 것으로 판단해서 함수를 인라인하기로 결정했다. 아래는 원래 사용되던 HistoryScreen에서 HistoryHeaderContainer를 인라인한 Presenter의 모습이다.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
const Presenter = (props: Props) => {
  const { getHistoryData, historyDataList, onPress, isLoading, count } = props;
  return (
    <Wrapper>
      <HeaderWrapper>
        <ListCountText>{`전체보기(${count}건)`}</ListCountText>
      </HeaderWrapper>
      <ListWrapper>
        <FlatList
          data={historyDataList}
          renderItem={renderItem(onPress)}
          refreshing={isLoading}
          onRefresh={getHistoryData}
          onEndReached={getHistoryData}
          onEndReachedThreshold={1}
          style={styles.flatList}
          contentContainerStyle={styles.container}
          keyExtractor={(item, index) => String(index)}
          ListEmptyComponent={() => <Text>아직 접수 내용이 없습니다.</Text>}
        />
      </ListWrapper>
    </Wrapper>
  );
};

이번 리팩토링에서 시도한 함수 인라인은 아주 간단한 리팩토링이었기 때문에 (책에서는 간단하지 않은 경우가 많다고 했지만!..) 이 정도만 적용 예시를 적는 걸로 마치고 다음 리팩토링 적용을 진행해보자.

후기

예시로 적은 것들 외에도 많은 리팩토링이 이루어질 수 있는 내용이었다. 기본적이지만, 적용할 거리가 많았다. 조금더 고급진 리팩토링 기법은 다음 장부터 나타나지 않을까 싶다.

리팩토링 실습하기

https://changhoi.kim/posts/etc/refactoring-practice/

Author

changhoi

Posted on

2020-06-19

Updated on

2020-06-19

Licensed under

#

관련 글

이 글이 마음에 드시나요? 다음을 통해 후원하실 수 있습니다:

댓글

팔로우

링크

카테고리

광고

Your browser is out-of-date!

Update your browser to view this website correctly.&npsb;Update my browser now

×

×