DEV ℧ Developer Diary

[Refactoring] 냄새 3. 긴 함수 (1)

20 Sep 2022

해당 포스트는 inflearn의 백기선님의 강의인 리팩토링 을 듣고 정리한 글입니다.

냄새 3. 긴 함수 (Long Function) (1)

  • 짧은 함수 vs 긴 함수
    • 함수가 길 수록 더 이해하기 어렵다 vs 짧은 함수는 더 많은 문맥 전환을 필요로 한다.
    • “과거에는” 작은 함수를 사용하는 경우에 더 많은 서브루틴 호출로 인한 오버헤드가 있었다.
    • 작은 함수에 “좋은 이름”을 사용했다면 해당 함수의 코드를 보지 않고도 이해할 수 있다.
    • 어떤 코드에 “주석”을 남기고 싶다면, 주석 대신 함수를 만들고 함수의 이름으로 “의도”를 표현해보자.
  • 사용할 수 있는 리팩토링 기술
    • 99%는 “함수 추출하기 (Extract Function)“로 해결할 수 있다. (* 함수추출하기의 Hint는 주석이다, 주석을 토대로 기능을 쪼개자)
    • 함수로 분리하면서 해당 함수로 전달해야 할 매개변수가 많아진다면 다음과 같은 리팩토링을 고려해볼 수 있다.
      • 임시 변수를 질의 함수로 바꾸기 (Replace Temp with Query)
      • 매개변수 객체 만들기 (Introduce parameter Object)
      • 객체 통째로 넘기기 (Preserve Whole Object)
    • 조건문이 너무 복잡해진다면, “조건문 분해하기 (Decompose Conditional)”를 사용해 조건문을 분리할 수 있다.
    • 같은 조건으로 여러개의 Switch 문이 있다면, “조건문을 다형성으로 바꾸기 (Replace Conditional with Polymorphism)”을 사용할 수 있다.
    • 반복문 안에서 여러 작업을 하고 있다면, 그 함수는 하나의 일을 하는것이 아니다, 그래서 하나의 메소드로 추출하기 어렵다면, “반복문 쪼개기 (Splict Loop)”를 적용할 수 있다.

예제코드는 repository의 참여자에 대한 참석율을 계산하여 마크다운파일로 추출하는 로직이다.

길다.. 먼저 print() 내부의 메소드에의 for문에서 참석율을 저장해서 이후 try{} 내부에서 마크다운파일로 저정하는 로직이다.

리팩토링 7. 임시 변수를 질의 함수로 바꾸기

  • 변수를 사용하면 반복해서 동일한 식을 계산하는 것을 피할 수 있고, 이름을 사용해 의미를 표현할 수도 있다.
  • 긴 함수를 리팩토링 할 때, 그러한 임시변수를 함수로 추출하여 분리한다면 빼낸 함수로 전달해야 할 매개변수를 줄일 수 있다.

예제 코드

  • Participant
public record Participant(String username, Map<Integer, Boolean> homework) {
    public Participant(String username) {
        this(username, new HashMap<>());
    }

    public double getRate(double total) {
        long count = this.homework.values().stream()
                .filter(v -> v == true)
                .count();
        return count * 100 / total;
    }

    public void setHomeworkDone(int index) {
        this.homework.put(index, true);
    }

}

public class StudyDashboard {

    public static void main(String[] args) throws IOException, InterruptedException {
        StudyDashboard studyDashboard = new StudyDashboard();
        studyDashboard.print();
    }

    private void print() throws IOException, InterruptedException {
        GitHub gitHub = GitHub.connect();
        GHRepository repository = gitHub.getRepository("whiteship/live-study");
        List<Participant> participants = new CopyOnWriteArrayList<>();

        int totalNumberOfEvents = 15;
        ExecutorService service = Executors.newFixedThreadPool(8);
        CountDownLatch latch = new CountDownLatch(totalNumberOfEvents);

        for (int index = 1 ; index <= totalNumberOfEvents ; index++) {
            int eventId = index;
            service.execute(new Runnable() {
                @Override
                public void run() {
                    try {
                        GHIssue issue = repository.getIssue(eventId);
                        List<GHIssueComment> comments = issue.getComments();

                        for (GHIssueComment comment : comments) {
                            String username = comment.getUserName();
                            boolean isNewUser = participants.stream().noneMatch(p -> p.username().equals(username));
                            Participant participant = null;
                            if (isNewUser) {
                                participant = new Participant(username);
                                participants.add(participant);
                            } else {
                                participant = participants.stream().filter(p -> p.username().equals(username)).findFirst().orElseThrow();
                            }

                            participant.setHomeworkDone(eventId);
                        }

                        latch.countDown();
                    } catch (IOException e) {
                        throw new IllegalArgumentException(e);
                    }
                }
            });
        }

        latch.await();
        service.shutdown();

        try (FileWriter fileWriter = new FileWriter("participants.md");
             PrintWriter writer = new PrintWriter(fileWriter)) {
            participants.sort(Comparator.comparing(Participant::username));

            writer.print(header(totalNumberOfEvents, participants.size()));

            participants.forEach(p -> {
                long count = p.homework().values().stream()
                        .filter(v -> v == true)
                        .count();
                double rate = count * 100 / totalNumberOfEvents;

                String markdownForHomework = String.format("| %s %s | %.2f%% |\n", p.username(), checkMark(p, totalNumberOfEvents), rate);
                writer.print(markdownForHomework);
            });
        }
    }

    /**
     * | 참여자 (420) | 1주차 | 2주차 | 3주차 | 참석율 |
     * | --- | --- | --- | --- | --- |
     */
    private String header(int totalEvents, int totalNumberOfParticipants) {
        StringBuilder header = new StringBuilder(String.format("| 참여자 (%d) |", totalNumberOfParticipants));

        for (int index = 1; index <= totalEvents; index++) {
            header.append(String.format(" %d주차 |", index));
        }
        header.append(" 참석율 |\n");

        header.append("| --- ".repeat(Math.max(0, totalEvents + 2)));
        header.append("|\n");

        return header.toString();
    }

    /**
     * |:white_check_mark:|:white_check_mark:|:white_check_mark:|:x:|
     */
    private String checkMark(Participant p, int totalEvents) {
        StringBuilder line = new StringBuilder();
        for (int i = 1 ; i <= totalEvents ; i++) {
            if(p.homework().containsKey(i) && p.homework().get(i)) {
                line.append("|:white_check_mark:");
            } else {
                line.append("|:x:");
            }
        }
        return line.toString();
    }
}

먼저 try 내부의 파일 생성 로직은 건드려서 매개변수를 줄여보도록 하자.

try (FileWriter fileWriter = new FileWriter("participants.md");
     PrintWriter writer = new PrintWriter(fileWriter)) {
    participants.sort(Comparator.comparing(Participant::username));

    writer.print(header(totalNumberOfEvents, participants.size()));

    participants.forEach(p -> {
        long count = p.homework().values().stream()
                .filter(v -> v == true)
                .count();
        double rate = count * 100 / totalNumberOfEvents;

        String markdownForHomework = String.format("| %s %s | %.2f%% |\n", p.username(), checkMark(p, totalNumberOfEvents), rate);
        writer.print(markdownForHomework);
    });
}

해당 로직은 참석자를 정렬 한 후, 참석자 만큼 반복문을 돌려서 String.format을 입혀 아래와 같이 출력한다.

| 참여자 (417) | 1주차 | 2주차 | 3주차 | 4주차 | 5주차 | 6주차 | 7주차 | 8주차 | 9주차 | 10주차 | 11주차 | 12주차 | 13주차 | 14주차 | 15주차 | 참석율 |
| --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- |
| 0417taehyun |:white_check_mark:|:white_check_mark:|:white_check_mark:|:x:|:x:|:x:|:x:|:x:|:x:|:x:|:x:|:x:|:x:|:x:|:x: | 20.00% |

markdown으로 출력하는 코드의 양식을 읽기가 힘드므로, 따로 메소드를 추출하여보면 더 수월하기 확인 할 수 있다.

String markdownForHomework = String.format("| %s %s | %.2f%% |\n", p.username(), checkMark(p, totalNumberOfEvents), rate);

중복되는 파라미터인 p, totalNumberOfEvents, rate를 매개변수로 빼준뒤 getFormat이라는 메소드로 추출해준다.

String markdownForHomework = getMarkdownForParticipant(totalNumberOfEvents, p, rate);

...

private String getMarkdownForParticipant(int totalNumberOfEvents, Participant p, double rate) {
    return String.format("| %s %s | %.2f%% |\n", p.username(), checkMark(p, totalNumberOfEvents), rate);
}

하지만, 매개변수가 3개가 되면 조금 많다고 느껴질 수 있으니, rate를 제거해주도록 해보자.

      try (FileWriter fileWriter = new FileWriter("participants.md");
           PrintWriter writer = new PrintWriter(fileWriter)) {
          participants.sort(Comparator.comparing(Participant::username));

          writer.print(header(totalNumberOfEvents, participants.size()));

          participants.forEach(p -> {
              long count = p.homework().values().stream()
                      .filter(v -> v == true)
                      .count();
              double rate = count * 100 / totalNumberOfEvents;

              String markdownForHomework = getMarkDownForParticipant(totalNumberOfEvents, p, rate);
              writer.print(markdownForHomework);
          });
      }

        ...

private String getMarkDownForParticipant(int totalNumberOfEvents, Participant p, double rate) {
    return String.format("| %s %s | %.2f%% |\n", p.username(), checkMark(p, totalNumberOfEvents), rate);
}

먼저 아래와 같이 getRate라는 함수로 추출해주자.

      try (FileWriter fileWriter = new FileWriter("participants.md");
           PrintWriter writer = new PrintWriter(fileWriter)) {
          participants.sort(Comparator.comparing(Participant::username));

          writer.print(header(totalNumberOfEvents, participants.size()));

          participants.forEach(p -> {
              double rate = getRate(totalNumberOfEvents, p);

              String markdownForHomework = getMarkDownForParticipant(totalNumberOfEvents, p, rate);
              writer.print(markdownForHomework);
          });
      }

        ...

private double getRate(int totalNumberOfEvents, Participant p) {
    long count = p.homework().values().stream()
        .filter(v -> v == true)
        .count();
    double rate = count * 100 / totalNumberOfEvents;
    return rate;
}

private String getMarkDownForParticipant(int totalNumberOfEvents, Participant p, double rate) {
    return String.format("| %s %s | %.2f%% |\n", p.username(), checkMark(p, totalNumberOfEvents), rate);
}

함수 추출 이후 자세히 살펴보면, getRate의 메소드와 getMarkDownForParticipant의 메소드는 rate를 제외하고는 동일한 매개변수를 사용하게 된다. 이후 getRate() 함수를 이용해 추출한 rate를 넘겨주는것이 아니라, getMarkDownForParticipant() 내부에서 직접 rate를 생성하면 매개변수를 하나 줄일 수 있게된다.

      try (FileWriter fileWriter = new FileWriter("participants.md");
           PrintWriter writer = new PrintWriter(fileWriter)) {
          participants.sort(Comparator.comparing(Participant::username));

          writer.print(header(totalNumberOfEvents, participants.size()));

          participants.forEach(p -> {
              String markdownForHomework = getMarkDownForParticipant(totalNumberOfEvents, p);
              writer.print(markdownForHomework);
          });
      }

        ...

private double getRate(int totalNumberOfEvents, Participant p) {
    long count = p.homework().values().stream()
        .filter(v -> v == true)
        .count();
    double rate = count * 100 / totalNumberOfEvents;
    return rate;
}

private String getMarkDownForParticipant(int totalNumberOfEvents, Participant p) {
    return String.format("| %s %s | %.2f%% |\n", p.username(), checkMark(p, totalNumberOfEvents), getRate(totalNumberOfEvents, p));
}

participants.forEach(p -> {}) 내부의 getRate()를 빼서 getMarkDownForParticipant()로 넘겨준 rate를 지우고 내부에서 직접 호출하였다.

리팩토링 8. 매개변수 객체 만들기

  • 같은 매개변수들이 여러 메소드에 걸쳐 나타난다면 그 메소드들은 밀접하게 관련이 있다는 뜻이다, 해당 메소드들은 그 매개변수들을 묶은 자료 구조를 만들 수 있다.
  • 그렇게 만든 자료구조는:
    • 해당 데이터간의 관계를 보다 명시적으로 나타낼 수 있다.
    • 함수에 전달할 매개변수 개수를 줄일 수 있다.
    • 도메인을 이해하는데 중요한 역할을 하는 클래스로 발전할 수도 있다.

두가지의 방법을 소개해보고자 한다.

8.1 중복되는 매개변수 객체 만들기

리팩토링을 통해 추출한 함수인 getRate와 getMarkDownForParticipant는 동일한 매개변수인 int totalNumberOfEvents, Participant p를 가지고 있다.

private double getRate(int totalNumberOfEvents, Participant p) {
    long count = p.homework().values().stream()
            .filter(v -> v == true)
            .count();
    double rate = count * 100 / totalNumberOfEvents;
    return rate;
}

private String getMarkDownForParticipant(int totalNumberOfEvents, Participant p) {
    return String.format("| %s %s | %.2f%% |\n", p.username(), checkMark(p, totalNumberOfEvents), getRate(totalNumberOfEvents, p));
}

int totalNumberOfEvents, Participant p의 매개변수를 ParticipantPrinter 라는 객체로 추출하여 전달하여 갯수를 줄이고, 보다 함수를 이해할 수 있도록 도와줄 수 있다.

        try (FileWriter fileWriter = new FileWriter("participants.md");
             PrintWriter writer = new PrintWriter(fileWriter)) {
            participants.sort(Comparator.comparing(Participant::username));

            writer.print(header(totalNumberOfEvents, participants.size()));

            participants.forEach(p -> {
                String markdownForHomework = getMarkDownForParticipant(new ParticipantPrinter(totalNumberOfEvents, p));
                writer.print(markdownForHomework);
            });
        }

    ...

private double getRate(ParticipantPrinter participantPrinter) {
    long count = participantPrinter.p().homework().values().stream()
            .filter(v -> v == true)
            .count();
    double rate = count * 100 / participantPrinter.totalNumberOfEvents();
    return rate;
}

private String getMarkDownForParticipant(ParticipantPrinter participantPrinter) {
    return String.format("| %s %s | %.2f%% |\n", participantPrinter.p().username(), checkMark(participantPrinter.p(), participantPrinter.totalNumberOfEvents()), getRate(new ParticipantPrinter(participantPrinter.totalNumberOfEvents(), participantPrinter.p())));
}

8.2 클래스의 중복되는 매개변수를 생성자로 빼기

클래스의 코드들을 보면 totalNumberOfEvents에 해당하는 필드가 정말 곳곳에서 쓰이고있다.

  • getRate(int totalNumberOfEvents, Participant p)
  • getMarkdownForParticipant(int totalNumberOfEvents, Participant p)
  • header(int totalNumberOfEvents, int totalNumberOfParticipants)
  • checkMark(Participant p, int totalEvents(totalNumberOfEvents))

거의 모든 메소드에서 사용된다고 보면 된다. 저 매개변수를 생성자를 통해 전역필드로 빼주자.

private final int totalNumberOfEvents;

public StudyDashboard(int totalNumberOfEvents) {
    this.totalNumberOfEvents = totalNumberOfEvents;
}

public static void main(String[] args) throws IOException, InterruptedException {
    StudyDashboard studyDashboard = new StudyDashboard(15);
    studyDashboard.print();
}

totalNumberOfEvents를 전역필드로 빼줌으로써 각 메소드에 전달되는 매개변수를 줄일 수 있을 뿐만 아니라, 전역변수를 통해 공통된 수를 사용 할 수 있게 된다.

  • 리팩토링 후
public class StudyDashboard {

    private final int totalNumberOfEvents;

    public StudyDashboard(int totalNumberOfEvents) {
        this.totalNumberOfEvents = totalNumberOfEvents;
    }

    public static void main(String[] args) throws IOException, InterruptedException {
        StudyDashboard studyDashboard = new StudyDashboard(15);
        studyDashboard.print();
    }

    private void print() throws IOException, InterruptedException {
        GitHub gitHub = GitHub.connect();
        GHRepository repository = gitHub.getRepository("whiteship/live-study");
        List<Participant> participants = new CopyOnWriteArrayList<>();

        ExecutorService service = Executors.newFixedThreadPool(8);
        CountDownLatch latch = new CountDownLatch(totalNumberOfEvents);

        for (int index = 1 ; index <= totalNumberOfEvents ; index++) {
            int eventId = index;
            service.execute(new Runnable() {
                @Override
                public void run() {
                    try {
                        GHIssue issue = repository.getIssue(eventId);
                        List<GHIssueComment> comments = issue.getComments();

                        for (GHIssueComment comment : comments) {
                            String username = comment.getUserName();
                            boolean isNewUser = participants.stream().noneMatch(p -> p.username().equals(username));
                            Participant participant = null;
                            if (isNewUser) {
                                participant = new Participant(username);
                                participants.add(participant);
                            } else {
                                participant = participants.stream().filter(p -> p.username().equals(username)).findFirst().orElseThrow();
                            }

                            participant.setHomeworkDone(eventId);
                        }

                        latch.countDown();
                    } catch (IOException e) {
                        throw new IllegalArgumentException(e);
                    }
                }
            });
        }

        latch.await();
        service.shutdown();

        try (FileWriter fileWriter = new FileWriter("participants.md");
             PrintWriter writer = new PrintWriter(fileWriter)) {
            participants.sort(Comparator.comparing(Participant::username));

            writer.print(header(participants.size()));

            participants.forEach(p -> {
                String markdownForHomework = getMarkdownForParticipant(p);
                writer.print(markdownForHomework);
            });
        }
    }

    private double getRate(Participant p) {
        long count = p.homework().values().stream()
                .filter(v -> v == true)
                .count();
        double rate = count * 100 / totalNumberOfEvents;
        return rate;
    }

    private String getMarkdownForParticipant(Participant p) {
        return String.format("| %s %s | %.2f%% |\n", p.username(), checkMark(p), getRate(p));
    }

    /**
     * | 참여자 (420) | 1주차 | 2주차 | 3주차 | 참석율 |
     * | --- | --- | --- | --- | --- |
     */
    private String header(int totalNumberOfParticipants) {
        StringBuilder header = new StringBuilder(String.format("| 참여자 (%d) |", totalNumberOfParticipants));

        for (int index = 1; index <= totalNumberOfEvents; index++) {
            header.append(String.format(" %d주차 |", index));
        }
        header.append(" 참석율 |\n");

        header.append("| --- ".repeat(Math.max(0, totalNumberOfEvents + 2)));
        header.append("|\n");

        return header.toString();
    }

    /**
     * |:white_check_mark:|:white_check_mark:|:white_check_mark:|:x:|
     */
    private String checkMark(Participant p) {
        StringBuilder line = new StringBuilder();
        for (int i = 1 ; i <= totalNumberOfEvents ; i++) {
            if(p.homework().containsKey(i) && p.homework().get(i)) {
                line.append("|:white_check_mark:");
            } else {
                line.append("|:x:");
            }
        }
        return line.toString();
    }
}

Related posts