긴 함수 (1)

 

함수의 길이에 관한 이야기로 함수의 코드의 길이는 어느 정도가 좋은가에 대해 다루는 것이 아니다. 

 

대신 나는 코드를 보는 관점에서 의도를 아는 것보다 구현을 이해하려고 애쓰는 자신을 발견하게 되면 

이는 긴 함수로 판단하여 의도를 표현할 수 있는 이름을 갖는 짧은 함수들로 구성하는 것이 맞다고 생각한다. 

 

과거의 언어가 아닌 이상 짧은 함수들로 이루어져 있어 많은 문맥 전환에 대해 콜 스택까지 신경 써야 하는 경우가 아니라면 

짧은 함수들로 분리하는 것도 나쁘지 않을 것이라고 생각한다.

 

 

이러한 리팩토링에는 거창한 기술은 필요 없다.

 

단지 함수를 추출하면 될 뿐이다. 

 

이 과정에서 만약 매개변수가 많아진다면 선택할 수 있는 리팩터링 기술은 존재한다. 

 

1. 임시 변수를 질의 함수로 변경하기

 

2. 매개변수 객체 만들기

 

3. 객체 통째로 넘기기 

 

조건문이 많다면 조건문을 분해하여 조건문을 분리할 수 있다. 

 

만약 같은 조건으로 여러 개의 switch 문이 있다면, 조건문을 다형성으로 바꾸는 것도 가능하다.

 

반복문 안에서 여러 작업을 하고 있어서 하나의 메서드로 추출하기 어렵다면 반복문을 쪼개는 기법도 사용할 수 있다.

 

 

예시를 보면서 하나씩 이해해보자.

 

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);
        });
    }
}

 

불행하게도 이러한 코드가 나에게도 존재한다.  print() 함수에서 의미를 찾기란 매우 어렵다.

더욱 어려운 건 비동기로 동작하고 있는 코드가 동작하고 많은 구현부들이 존재하여 이 코드를 읽을 때 

어쩔 수 없이 구현까지 전부 알아야 하는 수고스러움이 동반된다. 

 

 

실제 클라이언트 코드는 간단하다. DashBorad를 출력하라는 것이다.

 

StudyDashboard studyDashboard = new StudyDashboard();
studyDashboard.print();

 

가장 먼저 함수를 추출하면서 조금씩 리팩터링을 진행해보자. 

 

 

1. 임시 변수를 질의 함수로 변경하기 ( Replace Temp with Query )

 

변수를 사용하면 반복해서 동일한 식을 계산하는 것을 피할 수 있고, 이름을 사용해 의미를 표현할 수도 있다. 

긴 함수를 리팩터링 할 때, 그러한 임시 변수를 함수로 추출하여 분리한다면 빼낸 함수로 전달해야 할 매개변수를 줄일 수 있다. 

 

여기서 말하는 Query는 함수를 의미한다.

 

임시 변수를 만들어내는 표현식을 함수로 추출하자!

 

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);
    });
}

 

위 코드에서 markdownForHomework에 값을 할당하는 표현식을 보자.

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

 

이를 우리는 임시 변수를  Query, 즉 함수로 추출할 것이다.

 

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

 

 

 

그런데 파라미터가 3개 정도가 되면 많다고 느껴진다. 

매개변수를 없앨 수 있을까?

 

매개 변수도 임시 변수로 생각해보자. 그러면 해야 하는 일이 명확해진다. 

 

바로 매개 변수를 직접 전달하기보다는 getMarkdownForParticipant() 함수에서 직접 호출하도록 하면 되지 않을까?

 

다음과 같이 코드를 변경해보자.

 

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));
}

 

 

이를 판단하기 위해 우리는 고려해 보야할 것이 있는데,  함수 추출을 통해 함수를 추출했을 때 기본적인 매개변수들로 다른 매개변수를 생성할 수 있는 경우에 사용할 수 있다는 것이다. 

 

이러한 정보라면 파라미터를 제거하고 임시 변수를 쿼리로 질의하도록 코드를 변경할 수 있다.

 

 

2. 매개변수 객체 만들기

 

 

만약 같은 매개변수들이 여러 메서드에 걸쳐 나타난다면 그 매개변수들을 묶은 자료구조를 만들어 사용할 수 있다. 

 

1. 그렇게 만든 자료구조는 해당 데이터간의 관계를 보다 명시적으로 나타낼 수 있다

2. 함수에 전달할 매개변수 개수를 줄일 수 있다.

3. 도메인을 이해하는데 중요한 역할을 하는 클래스로 발전할 수도 있다.

 

사실 파라미터들이 중첩된다면 이 파라미터들은 밀접하게 관계가 있다고 볼 수 있습니다. 

 

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));
}

 

방금 임시 변수를 쿼리로 변경하는 부분에서 우리는 중첩되는 파라미터를 확인할 수 있습니다.

 

1. int totalNumberOfEvents

2. Participant p

 

이 둘을 합쳐 Parameter Object를 만들 수 있습니다. 

public record ParticipantPrinter(int totalNumberOfEvents, Participant p) {
}

 

그리고 해당 레코드에서 값을 가져오도록 메소드를 고칠 수 있을 것입니다.

 

혹은 totalNumberrOfEvents라는 값을 클래스 내부 필드로 사용해도 매개변수로 전달해줄 필요가 없습니다.

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

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

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

 

3. 객체 넘기기 ( Preserve Whole Object )

 

쉽게 생각해서 우리는 DTO를 넘겨버릴때가 많은 것 같습니다. dto에서 값을 하나하나 꺼내서 매개변수로 넘기기보다는 말이죠. 

 

저는 의존성을 생각해서 어떤 경우는 객체를 넘기고, 어떠한 경우에는 값을 하나하나 별도로 넘기고 있습니다. 이 과정에는 파라미터의 수가 적다면 사용합니다. 

 

이렇게 객체를 넘기게 되면 향후 미래에 추가될 매개변수까지 커버 할 수 있고 매개변수 목록을 크게 줄일 수 있습니다. 

 

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

 

해당 코드 처럼 분리하여 전달하는  p.username(), p.homework() 방법보다. 

 

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

다음 코드 처럼 객체를 넘겨서 처리하는 방법도 어쩌면 괜찮을 수 있습니다.

 

저는 이렇게 객체를 넘기는 경우 의존성에 대해 다시 한번 생각해 본다고 했습니다. 

 

여러 함수가 특별한 객체에 의존되어도 문제가 없는지. 이러한 함수들이 다른 도메인에서 사용될 여지가 있는가? 

인터페이스로 추출될 만한 것인지 확인하고 사용합니다. 이럴 경우는 low 타입으로 사용하는 게 조금 더 범용적이기 때문입니다. 

 

그리고 객체지향적으로 더 나아가보면 과연 Participant 객체를 의존하는 함수의 위치가 여기가 맞는가에 대해 의문점이 발생해야 합니다.

 

그 이유는 participant, 즉 의존하는 객체를 사용하는 로직인데 그 로직이 왜 외부에 있지? 해당 객체가 제공해야 하는 게 아닐까? 

 

이러한 의미가 있다면 해당 메서드를 해당 객체로 옮겨서 해당 기능을 제공하는 방법도 생각해보아야 합니다.

 

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

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

 

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

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

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


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

 

이제 레코드에 해당 getRate 메소드를 옮겨 사용할 수 있습니다. 만약 해당 레코드가 하는 일이 많다면 클래스로 변경하는 것도 고려해보아야 합니다.