긴 함수 (2)

 

긴 함수 (1)

긴 함수 (1)

 

 

함수에 대해 리팩터링 하는 방법에 대해 (1)에서 소개와 몇 가지 방법을 소개하는데 추가적인 방법에 대해 적어보자.

 

 

커맨드 패턴은 함수를 독립적인 객체로 만드는 패턴이다. 

이를 통해 undo 기능도 추가적으로 만들 수 도 있었다. 

 

함수를 명령으로 바꾸기 ( Replace Function with Command )

 

이 방법도 커맨드 패턴과 비슷하게 독립적인 객체로 만드는 방법이다. 

 

커맨드 역할을 하는 함수를 독립적인 객체로 만들면 클래스 필드를 활용하거나 함수를 더 작게 분리하여 복잡성을 낮출 수 있습니다. 

 

클래스로 분리함으로써 상속이나 템플릿을 활용할 수도 있습니다. 

다만 단점은 그만큼 클래스 구조에 대한 복잡성은 증가할 수 있습니다. 

 

그래서 대부분 함수를 분리하는 방법을 사용하지만 분리해도 해당 코드 위치가 애매모호하고 향후 더 복잡해질 수 있다면 커맨드 패턴을 적용하는 것도 좋은 방법 중 하나이다.

 

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) {
                        Participant participant = findParticipant(comment.getUserName(), participants);
                        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);
        });
    }
}

 

이 함수에서 결과를 md 파일로 도출하는 부분에 있어서 다른 확장자로 출력하는 기능을 염두에 두었기 때문에 

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

 

다음 코드를 커맨드 패턴을 적용해보자.

 

public class StudyPrinter {

    
    
    public void execute(List<Participant> participants) throws IOException {
        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 String getMarkdownForParticipant(Participant p) {
        return String.format("| %s %s | %.2f%% |\n", p.username(), checkMark(p, this.totalNumberOfEvents),
                p.getRate(this.totalNumberOfEvents));
    }

    private String header(int totalNumberOfParticipants) {
        StringBuilder header = new StringBuilder(String.format("| 참여자 (%d) |", totalNumberOfParticipants));

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

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

        return header.toString();
    }

}

 

다음과 같이 사용되는 코드를 분리함에 따라 해당 함수만 사용하는 여러 메서드들도 동시에 이동하게 된다.

 

이 과정에서 우리는 빨간 글씨에 필요한 필드들을 확인할 수 있다.

 

public class StudyPrinter {

    private int totalNumberOfEvents;
    private List<Participant> participants;

    public StudyPrinter(int totalNumberOfEvents, List<Participant> participants) {
        this.totalNumberOfEvents = totalNumberOfEvents;
        this.participants = participants;
    }

    public void execute() throws IOException {
        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 String getMarkdownForParticipant(Participant p) {
        return String.format("| %s %s | %.2f%% |\n", p.username(), checkMark(p, this.totalNumberOfEvents),
                p.getRate(this.totalNumberOfEvents));
    }

    private String header(int totalNumberOfParticipants) {
        StringBuilder header = new StringBuilder(String.format("| 참여자 (%d) |", totalNumberOfParticipants));

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

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

        return header.toString();
    }

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

 

이제 완전히 독립적인 커맨드 클래스로 구성되었다. 

 new StudyPrinter(this.totalNumberOfEvents, participants).execute();

이제 사용할 때는 다음과 같이 사용할 수 있게 되었다. 

 

이제 상위 인터페이스로 만들고 각각의 구현체를 만들어서 사용할 수 있도록 유연성도 가지게 되었고 

여러 필드들을 추상 클래스로 올려서 더욱 깔끔하게 리팩터링 할 수 도 있겠다고 생각할 수 있다. 

 

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) {
                        Participant participant = findParticipant(comment.getUserName(), participants);
                        participant.setHomeworkDone(eventId);
                    }

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

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

    new StudyPrinter(this.totalNumberOfEvents, participants).execute();
}

 

실제 함수도 매우 간략하게 변하였다.

 

 

조건문 분해하기 ( Decompose Conditional)

 

여러 조건에 따라 달라지는 코드를 작성하다 보면 긴 함수가 만들어지는 것을 볼 수 있는다.

 

private Participant findParticipant(String username, List<Participant> participants) {
    Participant participant;
    if (participants.stream().noneMatch(p -> p.username().equals(username))) {
        participant = new Participant(username);
        participants.add(participant);
    } else {
        participant = participants.stream().filter(p -> p.username().equals(username)).findFirst().orElseThrow();
    }

    return participant;
}

 

나는 조건문을 볼때 우선적으로 진행하는 것은 조건이 매우 길다면 이를 별도의 의미 있는 함수명을 가진 함수로 뽑아낸다. 

 

participants.stream().noneMatch(p -> p.username().equals(username))

 

마치 다음과 같은 코드가 조건에 있으면 의도 대신 구현을 읽어야 하기 때문이다. 

 

private Participant findParticipant(String username, List<Participant> participants) {
    Participant participant;
    if (isNewParticipant(username, participants)) {
        participant = createNewParticipant(username, participants);
    } else {
        participant = findExistingParticipant(username, participants);
    }

    return participant;
}

 

이렇게 조건문을  Decompose하고 나면 코드를 더욱 간추릴 수 있다.

 

private Participant findParticipant(String username, List<Participant> participants) {
    return isNewParticipant(username, participants) ?
            createNewParticipant(username, participants) :
            findExistingParticipant(username, participants);
}

나이스 한 삼항 연산자이다. 

 

private Participant findParticipant(String username, List<Participant> participants) {
    Participant participant;
    if (participants.stream().noneMatch(p -> p.username().equals(username))) {
        participant = new Participant(username);
        participants.add(participant);
    } else {
        participant = participants.stream().filter(p -> p.username().equals(username)).findFirst().orElseThrow();
    }

    return participant;
}

 

기존 코드를 보고 리팩터링 된 코드를 본다면 매우 많은 코드가 없어진 것을 확인할 수 있다.

이 와중에 우리는 반복적인 매개변수를 확인했다면 또 별도의 클래스로 분리해낼 수도 있다. 

 

사실 리팩토링을 하기 전에는 해당 코드가 어떻게 변할지 정확히 예상하는 것은 많은 노련한 개발자가 아니고선 상상하기 힘들다. 

 

하나씩 적용하다보니 점진적으로 깔끔해지는 코드를 보는 것일 뿐이다.

 

반복문 쪼개기 ( Split Loop )

 

하나의 반복문에서 여러 다른 작업을 하는 코드를 쉽게 찾아볼 수 있다. 

 

당연히 한번 반복문을 돌면서 할 수 있는 작업들을 하는 것이 매우 이상적으로 보인다.

 

그런데 코드를 수정할 때 반복문 하나가 매우 길다면 모든 작업을 고려하면서 작업을 해야 한다. 

그래서 만약 이 반복문 자체가 성능에 대한 문제가 없다면 

반복문을 자체로 분리하고 쪼개고 나눠놓고 다시 성능적으로 문제인지 확인해보아도 된다. 

 

그리고 성능 최적화와 리팩터링은 별개의 작업이기 때문에 리팩터링 이후 성능 최적화를 시도해도 된다.

 

 

 

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

                for (GHIssueComment comment : comments) {
                    Participant participant = findParticipant(comment.getUserName(), participants);
                    participant.setHomeworkDone(eventId);

                    if (firstCreatedAt == null || comment.getCreatedAt().before(firstCreatedAt)) {
                        firstCreatedAt = comment.getCreatedAt();
                        first = participant;
                    }
                }

                firstParticipantsForEachEvent[eventId - 1] = first;
                latch.countDown();
            } catch (IOException e) {
                throw new IllegalArgumentException(e);
            }
        }
    });
}

 

루프를 쪼개보자.

 

다음과 같이 사전 작업을 진행해보자.

 

for (GHIssueComment comment : comments) {
    Participant participant = findParticipant(comment.getUserName(), participants);
    participant.setHomeworkDone(eventId);
}

Date firstCreatedAt = null;
Participant first = null;
for (GHIssueComment comment : comments) {
    Participant participant = findParticipant(comment.getUserName(), participants);

    if (firstCreatedAt == null || comment.getCreatedAt().before(firstCreatedAt)) {
        firstCreatedAt = comment.getCreatedAt();
        first = participant;
    }
}

 

두 개의 반복문을 만들고 관련 있는 필드를 모은 다음 이제 메서드로 추출할 것이다.

 

private Participant findFirst(List<GHIssueComment> comments) throws IOException {
    Date firstCreatedAt = null;
    Participant first = null;
    for (GHIssueComment comment : comments) {
        Participant participant = findParticipant(comment.getUserName(), participants);

        if (firstCreatedAt == null || comment.getCreatedAt().before(firstCreatedAt)) {
            firstCreatedAt = comment.getCreatedAt();
            first = participant;
        }
    }
    return first;
}
private void checkHomework(List<GHIssueComment> comments, int eventId) {
    for (GHIssueComment comment : comments) {
        Participant participant = findParticipant(comment.getUserName(), participants);
        participant.setHomeworkDone(eventId);
    }
}

 

효율적이지 않다고 생각할 수 있다. 왜 동시에 처리할 수 있는 일을 분리해야 하는가에 대해 의문을 갖고 성능을 문제 삼을 수 있다.

API 호출하는 부분인 findParticipant를 보면서 네트워크 비용을 더 많이 쓴다고 할 수 있다. 

이는 각각의 상황에 맞게 고려해야 하는 부분일 수 있다.  왜냐하면 이로 인해 얻을 수 있는 장점은 가독성이기 때문이다.

 

private void print() throws IOException, InterruptedException {
    GHRepository ghRepository = getGhRepository();

    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 = ghRepository.getIssue(eventId);
                    List<GHIssueComment> comments = issue.getComments();
                    checkHomework(comments, eventId);
                    firstParticipantsForEachEvent[eventId - 1] = findFirst(comments);
                    latch.countDown();
                } catch (IOException e) {
                    throw new IllegalArgumentException(e);
                }
            }
        });
    }
    latch.await();
    service.shutdown();

    new StudyPrinter(this.totalNumberOfEvents, this.participants).execute();
    printFirstParticipants();
}

 

그렇지만 코드를 유지 보수하는 입장에서 가독성은 매우 중요한 코드 품질의 지표가 된다.

 

이로써 루프 쪼개기는 마무리가 되었다. 

 

리팩터링의 장점은 더욱이 발전할 수 있다는 것이다. 다음 코드는 어떤가?

private void print() throws IOException, InterruptedException {
    checkGithubIssues(getGhRepository());
    new StudyPrinter(this.totalNumberOfEvents, this.participants).execute();
    printFirstParticipants();
}
private void checkGithubIssues(GHRepository ghRepository) throws InterruptedException {
    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 = ghRepository.getIssue(eventId);
                    List<GHIssueComment> comments = issue.getComments();
                    checkHomework(comments, eventId);
                    firstParticipantsForEachEvent[eventId - 1] = findFirst(comments);
                    latch.countDown();
                } catch (IOException e) {
                    throw new IllegalArgumentException(e);
                }
            }
        });
    }
    latch.await();
    service.shutdown();
}

 

 

 

ps - 요즘 병렬 프로그래밍하고 있는데 잠시 참고해보자.

CountDownLatch, CopyOnWriteArrayList

 

 

조건문을 다형성으로 바꾸기 ( Replace Conditional with Polymorphism)

 

 

여러 타입에 따라 각기 다른 로직으로 처리해야 하는 경우에 다형성을 적용해서 조건문을 보다 명확하게 분리할 수 있다. 

 

이렇게 반복되는 switch문을 각기 다른 클래스를 만들어 제거할 수 있다.

 

공통으로 사용되는 로직은 상위 클래스에 두고 달라지는 부분만 하위 클래스에 둠으로써, 달라지는 부분만 강조할 수 있다.

 

private void print() throws IOException, InterruptedException {
    checkGithubIssues(getGhRepository());
    new StudyPrinter(this.totalNumberOfEvents, this.participants, PrinterMode.MARKDOWN).execute();
}

 

다음과 같이 PrinterMod의 변경에 따라 출력되는 형태가 달라지도록 구현하려고 한다. 

public void execute() throws IOException {
    switch (printerMode) {
        case CVS -> {
            try (FileWriter fileWriter = new FileWriter("participants.cvs");
                 PrintWriter writer = new PrintWriter(fileWriter)) {
                writer.println(cvsHeader(this.participants.size()));
                this.participants.forEach(p -> {
                    writer.println(getCvsForParticipant(p));
                });
            }
        }
        case CONSOLE -> {
            this.participants.forEach(p -> {
                System.out.printf("%s %s:%s\n", p.username(), checkMark(p), p.getRate(this.totalNumberOfEvents));
            });
        }
        case MARKDOWN -> {
            try (FileWriter fileWriter = new FileWriter("participants.md");
                 PrintWriter writer = new PrintWriter(fileWriter)) {

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

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

이 때문에 execute()에서 상수에 따라 분기를 해야 하기 때문에 코드가 조금 길어진 것을 확인할 수 있다.

 

우리는 switch문에 대해 별도의 클래스 추출을 고려해볼 수 있다 이를 위해 해야 하는 작업은 다음과 같다.

 

1. 해당 메서드를 추상 메서드로 만든다.

 

public abstract void execute() throws IOException;

 

이를 통해 자연스레 클래스는 추상 클래스가 될 것이다. 

 

2. 마치 커맨드 패턴처럼 클래스를 정의한다.

 

 

개수만큼 해당 클래스를 정의해 필요에 따라 구성한다. 

 

private void print() throws IOException, InterruptedException {
    checkGithubIssues(getGhRepository());
    new MarkdownPrinter(this.totalNumberOfEvents, this.participants).execute();
}

 

이제 필요에 따라 생성되는 객체만 달리하면 된다.

 

만약 동적으로 각기 다른 인스턴스가 필요하다면 팩토리 메서드를 만들어서 런타임에 변경할 수도 있다.