리팩터링 2판 - 05. 코드에서 나는 악취 (코드스멜)

오늘은 리팩터링 2판 3단원의 코드스멜 단원을 읽으며 드는 생각을 나눠보려해요~

그리고 내용을 읽어보면서 팀문화에 녹여낼 수 있다면 좋겠다는 점도 써보고싶어요!

 

기이한 이름

기이한 이름 단원에서 설명하고자하는 바는 N년차 개발자들의 최대 고민인 변수(함수)명 짓기에 해당해요.

해당 파일을 작성하고 있는 본인은 당연하다고 생각한 "이름 짓기"도 처음보는 사람 입장에선 전혀 이해가 안가는 경우가 있을거에요.

 

저는 최대한 코드의 맥락을 통해 어떤 역할을 담당하는지 나타내고자 스스로 "리팩터링"을 진행하며, “코드리뷰”를 통해 다시 한번 체크하고 넘어가고 있어요!

 

대표적으로 아래와 같은 함수를 리팩터링을 통해 효과를 얻을 수 있어요!

<div>{getDurationTimeBy(distance)}</div> // 결과값 : 00분


const getDurationTimeBy = (distance:number) => {
	return `Math.ceil(distance * 0.9 / 60)분`
}

기존엔 convertDistance로 간단하게 작성하고 넘어갔지만, 함수를 통해 어떤 것을 얻고 싶은지에 대해 알 수 없었어요.

결과적으로 저는 getDurationTimeBy(distance) 로 리팩터링하면서 걸리는 시간을 얻는 함수임을 인지하기 쉬워졌어요!

 

생각보다 우리 주변엔 이해하기 어려운 코드가 많아요.

그렇기때문에 저는 항상 보이스카우트 원칙에 따라 기능을 추가하고자 하는 파일에 접근 시에 함수명을 다시 한번 읽어보며 리팩터링한 후 시작하고 있어요!

 

중복 코드

똑같은 코드 구조가 반복된다면 하나로 통합하세요!

React를 사용한다면 가장 대표적인 컴포넌트화를 통한 재사용성이 이 부분에 해당하지 않을까요?!

 

저는 최근 테스트 코드를 작성하면서 굉장히 중복 코드를 단축시켜 개선했던 경험이 있어요!

describe('1', () => {
  ...
  
  it('1-1', () => {
    const customReview = reviews.slice(0, 1);
    server.use(fetchReviews(customReview));
    render(<HomePage serversideReviews={shakeReviews(customReview)} />);
    
    ...
  });

  it('1-2', () => {
    const customReview = reviews.slice(0, 2);
    server.use(fetchReviews(customReview));
    render(<HomePage serversideReviews={shakeReviews(customReview)} />);
    
    ...
  });
  
  it('1-3', () => {
    const customReview = reviews.slice(0, 3);
    server.use(fetchReviews(customReview));
    render(<HomePage serversideReviews={shakeReviews(customReview)} />);
    
    ...
  });
  
  ...
});

초기에는 이렇게 테스트케이스마다 server.use를 통해 msw mocking을 한 후, serverside로 불러오는 곳에도 넣어줘야하는 반복되는 패턴이 보였어요.

 

해당 it 구문이 8개가 되다 보니 코드가 거의 260줄에 가까웠지만 다음과 같이 개선할 수 있었어요.

describe('1', () => {

  ...
  
  const renderHomePage = (reviews) => {
  	server.use(fetchReviews(reviews));
    render(<HomePage serversideReviews={shakeReviews(reviews)} />);
  }
  
  it('1-1', () => {
  	renderHomePage(reviews.slice(0, 1))
    
    ...
  });

  it('1-2', () => {
  	renderHomePage(reviews.slice(0, 2))
    
    ...
  });
  
  it('1-3', () => {
  	renderHomePage(reviews.slice(0, 3))
    
    ...
  });
  ...
});

 

이렇게 render 함수를 함수화하여 사용하면 중복된 코드를 제거할 수 있었어요

결과적으로 위의 코드는 260줄에서 180줄로 대폭 줄이게 되었다니 너무 깔끔하죠?!

 

저는 항상 이 말을 생각하면서 개발하고 있어요.

처음부터 좋은 코드는 없지만, 리팩터링을 통해 좋은 코드로 만들어갈 수 있다.

 

반응형

긴 함수

하나의 함수가 너무 많은 역할을 담당하면 이해하기 어렵고, 재사용성이 떨어져요.

적절한 SRP 원칙을 통해 함수안에서의 동작들이 어떤 의도를 갖고 진행되는지를 나타내는게 중요해요! (무작정 긴 함수의 길이를 줄이라는게 핵심이 아니라, 길더라도 이해하기 좋게 읽히는 코드를 짜라는 뜻!)

 

이번엔 저의 개인 프로젝트인 exambomb 문제은행 서비스 ( https://exam-bomb-service.vercel.app )에서 코드를 가져와 예시를 들어보려해요. 바로 아래 예시에선 테스트결과를 fetch 보내는 함수가 쭉 작성되어있어요.

const onPostAsnwer = (idx: number) => async () => {
  try {
    const isCorrect = currentQuestionDetailId === answers[idx].questionDetailId;

    if (isInitialTest) {
      await fetcher<PatchUserTestDetailInit>('/user/test/detail', {
        method: 'PATCH',
        headers: {
          'Content-Type': 'application/json',
          authorization: session?.user?.accessToken,
        },
        body: JSON.stringify({
          userTestId: currentQuestion.userTestId,
          userTestDetailId: currentQuestion.userTestDetailId,
          answerYn: isCorrect ? 'Y' : 'N',
          questionDetailId: answers[idx].questionDetailId,
        }),
      });
    }
    
    if (answers.every((a) => a.submitYn === 'Y')) {
      alert(`시험이 끝났습니다!`);
      setIsFinishTest(true);
    }
  } catch (err) {
    console.log(err);
  }
};

여기서 목적과 구현에 맞게 긴 함수를 분리한다면 아래처럼 update를 하는 함수를 빼내어 좀 더 무엇을 하는지 명확하게 나타낼 수 있어요.

  const onPostAsnwer = (idx: number) => async () => {
      try {
        const isCorrect =
          currentQuestionDetailId === answers[idx].questionDetailId;

        if (isInitialTest) {
          await updateTestDetail({
            userTestId: currentQuestion.userTestId,
            userTestDetailId: currentQuestion.userTestDetailId,
            answerYn: isCorrect ? 'Y' : 'N',
            questionDetailId: answers[idx].questionDetailId,
          });
        }
        
        if (answers.every((a) => a.submitYn === 'Y')) {
          alert(`시험이 끝났습니다!`);
          setIsFinishTest(true);
        }
      } catch (err) {
        console.log(err);
      }
}

 

리팩터링에선 전체 코드의 양이 길어지는건 크게 중요하게 생각하지 않고, 결과적으로 얼마나 이해하기 쉽게 나타낼 것인가에 초점을 두고 있어요. 함께 더 좋은 코드를 만들어 가봐요!

 

오늘은 여기까지하고 이어서 또 오도록 하겠습니다. 재밌게 읽어주세요~