[클린코드 적용기] 4. Flag Parameter

이 글에서는 클린코드 3장 함수에서 다룬 플래그 인수 관련 리팩토링을 다룹니다.

플래그 함수를 피해야 하는 이유?

책에서는 함수에 플래그 인수를 넘기는 것에 대해 하기의 단점을 언급했습니다.

• 플래그 인수를 넘기면, 어떤 동작을 하는지 직관적으로 이해하기 어렵습니다.

  render(true);
  render(false);

  • 위처럼 의미가 명확히 전달되지 않아 true, false에 따라 함수 동작이 어떻게 달라지는지 이해하려면 내부 코드를 이해해야만 합니다.
• 그 함수가 여러 가지 일을 처리하고 있다는 의미이므로, 그만큼 변경 요인이 다양해짐을 의미합니다.

적용

클라이언트 코드에서 함수의 의미를 알기 어려울 뿐더러, 메서드 내부 코드 자체도 필수적으로 분기문을 넣어 처리해야 하므로 함수를 분리하는 것이 더 좋은 코드라는 의견에 공감했습니다. 다음은 유저의 테스트 관련 통계를 조회하는 메서드입니다.

public ReadUserTestingStats readUserTestingStats(Long targetUserId, Long myUserId, String type) {
    User targetUser = findUserByIdOrThrowIfNonexistentUser(targetUserId);
    int uploadBoardCount = getUploadBoardCountBy(targetUserId);
    ReadRankQuery readRankQuery = getRankAndCompletedTestStatsBy(targetUserId);

    int completedTestCount = 0;
    Integer rank = null;

    if (readRankQuery != null) {
        completedTestCount = readRankQuery.getCompletedTestCount();
        rank = readRankQuery.getRank();
    }

    String nickname = targetUser.getNickname();
    
    // 다른 유저 정보의 경우에는 서로 도움 횟수까지 Response
    if (type.equals(USER_INFO_REQUEST_TYPE_ANOTHER)) {
        long interactionCounts = userInteractionStatusRepository.countUserInteractionStatusByUserId(myUserId, targetUserId);
        return new ReadUserTestingStats(completedTestCount, uploadBoardCount, rank, nickname, interactionCounts);
    }

    return new ReadUserTestingStats(completedTestCount, uploadBoardCount, rank, nickname, null);
}

• boolean 타입의 인수는 아니지만, type이 그 역할을 수행하고 있습니다.
• 다른 유저에 대한 테스트 관련 통계를 가져올 때는 그 대상과 나와의 상호작용 횟수까지 추가로 조회하여 데이터를 전달하는 것입니다.
• 제가 생각했을 때 단점은 2가지가 있을 것 같습니다.
  1. 플래그 인수를 사용하는 것보다 가독성은 좋겠지만, type을 위해 상수를 별도로 관리해야 하는 소요가 발생합니다. 하지만, 이마저도 함수 명으로 의미를 구분하는 것 보다는 덜 직관적인 것 같습니다.
  2. 만약 다른 유저 조회 시 상호작용 횟수 뿐 아니라 다른 데이터도 추가로 보내주고 싶다면, 자신을 조회하는 로직에는 변화가 없었음에도 위 메서드를 수정해야 합니다.
• 굳이 type으로 특정 상황에만 별도의 작업을 수행하지 않고, 함수를 분리한다면?

  public ReadUserTestingStats readMyTestingStats(Long userId) {
      User user = findUserByIdOrThrowIfNonexistentUser(userId);
  
      return readUserTestingStats(user, null);
  }
  
  public ReadUserTestingStats readAnotherTestingStats(Long targetUserId, Long myUserId) {
      User targetUser = findUserByIdOrThrowIfNonexistentUser(targetUserId);
      long interactionCounts = getInteractionCountBy(targetUserId, myUserId);
  
      return readUserTestingStats(targetUser, interactionCounts);
  }
  
  public ReadUserTestingStats readUserTestingStats(User user, Long interactionCounts) {
      int uploadBoardCount = getUploadBoardCountBy(user.getId());
      ReadRankQuery readRankQuery = getRankAndCompletedTestStatsBy(user.getId());
  
      int completedTestCount = extractCompletedTestCount(readRankQuery);
      Integer rank = extractRank(readRankQuery);
      String nickname = user.getNickname();
  
      return new ReadUserTestingStats(completedTestCount, uploadBoardCount, rank, nickname, interactionCounts);
  }

  • 상기 단점 2가지를 모두 보완할 수 있었습니다.
    1. 함수 이름만으로 더 직관적인 의미 전달이 가능해졌습니다.
    2. 다른 유저 조회 시 전달 데이터가 바뀌더라도 readAnotherTestingStats메서드만 수정하면 됩니다.

회고

위 리팩토링 과정을 진행해 보면서 하나의 역할을 하는 함수라면 네이밍도 쉬워질 뿐 아니라, 의미 전달도 명확해 진다고 느껴졌습니다. 또한, 함수를 분리하여 공통적인 부분은 별도의 함수를 만들어 재사용이 가능하니 더 간결한 함수 작성이 가능했습니다. 함수가 간결해지고, 의미가 명확해지니 코드 상으로 자신을 조회할 때와 다른 유저를 조회할 때의 차이점도 빠르게 확인할 수 있었습니다.

피드백은 언제나 환영입니다! 🙇🏻‍♂️