DEV ℧ Developer Diary

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

28 Sep 2022

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

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

리팩토링 12. 반복문 쪼개기

  • 하나의 반복문에서 여러 다른 작업을 하는 코드를 쉽게 찾아볼 수 있다.
  • 해당 반복문을 수정할 때 내부 로직이 복잡하다면 여러 작업을 모두 고려하며 코딩을 해야한다.
  • 반복문을 여러개로 쪼개면 보다 쉽게 이해하고 수정할 수 있다.
  • 성능 문제를 야기할 수 있지만 “리팩토링”은 “성능 최적화”와 별개의 작업이다. 리팩토링을 마친 이후에 성능 최적화를 시도할 수 있다.

예제코드

print 함수중 new Runnable()을 @Override해서 구현한 내부 for문을 리팩토링을 해보도록하자. 해당 소스는 Github api를 이용해 과제를 제출한 사람과 누가 먼저 제출했는지를 체크하여 출력하는 함수이다.

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

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

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

먼저 아래의 for문은 하나의 for문 구역안에 두가지 작업을 진행하고 있다.

  1. 과제를 제출한 사람을 체크
  2. 가장 먼저 과제를 제출한 사람을 체크
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;
    }
}

먼저 for문 안에서 진행하고 있는 두가지 일을 한가지 일만 하도록 분리시켜 준다.

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

그리고 각각의 for문을 함수로 분리시켜 주도록 하자. 그러면 반복문을 두개의 함수로 분리 시킬 수 있다.

checkHomework(comments, eventId);
firstParticipantsForEachEvent[eventId - 1] = findFirst(comments);

...

private void checkHomework(List<GHIssueComment> comments, int eventId) {
    for (GHIssueComment comment : comments) {
        Participant participant = findParticipant(comment.getUserName(), participants);
        participant.setHomeworkDone(eventId);
    }
}

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

그러면 print 함수를 다시 살펴보자. 숙제 제출 체크로직과 첫번째로 제출한 사람을 체크하는 로직을 분리함 으로써 가독성은 조금 늘었지만, 아직 print() 내부가 복잡하다. ExecutorService 와 CountDownLatch 는 MultiThread 로직을 위해 필요한 필드로 바로 밑 for문 내부에서 작업하는 로직에 필요한 필드이다.

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

MultiThread부분을 묶어서 조금더 가독성있게 수정 할 수 있다.

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

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

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

리팩토링 13. 조건문을 다형성으로 바꾸기

  • 여러 타입에 따라 각기 다른 로직으로 처리해야 하는 경우에 다형성을 적용해서 조건문을 보다 명확하게 분리할 수 있다. (예. 책, 음악, 음식 등…) 반복되는 switch문을 각기 다른 클래스를 만들어 제거 할 수있다.
  • 공통으로 사용되는 로직은 상위 클래스에 두고 달라지는 부분만 하위 클래스에 둠으로써, 달라지는 부분만 강조 할 수 있다.
  • 모든 조건문을 다형성으로 바꿔야 하는 것은 아니다.

예제코드

클래스들이 굉장히 길어 주요 함수만 뽑아서 작성하도록 했다. StudyDashboard의 print함수에 PrinterMode에서 주는 mode에 따라 StudyPrinter의 내부 실행이 달라지도록 하는 함수이다.

  • PrinterMode
public enum PrinterMode {
    CONSOLE, CVS, MARKDOWN
}
  • StudyDashboard
private void print() throws IOException, InterruptedException {
    checkGithubIssues(getGhRepository());
    new StudyPrinter(this.totalNumberOfEvents, this.participants, PrinterMode.MARKDOWN).execute();
}
  • StudyPrinter
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);
                });
            }
        }
    }
}

기본적으로 StudyPrinter는 상위클래스로 유지하되, CSV, CONSOLE, MARKDOWN을 각각 출력할 수 있는 하위 클래스를 만들어 상속 받도록 하자.

각각의 하위 클래스를 만들어주면, StudyPrinter 내부의 PrinterMode는 필요가 없어지게 된다.

  • ConsolePrinter
public class ConsolePrinter extends StudyPrinter{
    public ConsolePrinter(int totalNumberOfEvents, List<Participant> participants) {
        super(totalNumberOfEvents, participants);
    }
}
  • CsvPrinter
public class CsvPrinter extends StudyPrinter{
    public CsvPrinter(int totalNumberOfEvents, List<Participant> participants) {
        super(totalNumberOfEvents, participants);
    }
}
  • MarkdownPrinter
public class MarkdownPrinter extends StudyPrinter{
    public MarkdownPrinter(int totalNumberOfEvents, List<Participant> participants) {
        super(totalNumberOfEvents, participants);
    }
}

이후 PrinterMode로 구분했던 StudyPrinter 내부의 execute를 쪼개서 각각의 하위클래스에 Override 해준다. 그리고 getCvsForParticipant, cvsHeader, checkMark 등등.. StudyPrinter에서 사용했던 메소드들을 역할 군에 맞게 쪼개서 분리해준다.

분리하기에 앞서 StudyPrinter 내부에 있던 필드 totalNumberOfEvents, participants 는 private에서 protected로 변경해 주어야 다른 클래스에서 공유받아 사용이 가능하다.

여기서 MarkdownPrinter와 ConsolePrinter의 checkMark는 그대로 사용은 가능하지만 공통적으로 사용되므로 각각의 하위클래스에서는 삭제해주고 상위 클래스에서 protected로 변경 해주어 같이 사용할 수 있게 해주자.

protected int totalNumberOfEvents;
protected List<Participant> participants;
  • ConsolePrinter
public class ConsolePrinter extends StudyPrinter{
    public ConsolePrinter(int totalNumberOfEvents, List<Participant> participants) {
        super(totalNumberOfEvents, participants);
    }

    @Override
    public void execute() throws IOException {
        this.participants.forEach(p -> {
            System.out.printf("%s %s:%s\n", p.username(), checkMark(p), p.getRate(this.totalNumberOfEvents));
        });
    }

}
  • CsvPrinter
public class CsvPrinter extends StudyPrinter{
    public CsvPrinter(int totalNumberOfEvents, List<Participant> participants) {
        super(totalNumberOfEvents, participants);
    }

    @Override
    public void execute() throws IOException {
        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));
            });
        }
    }

    private String getCvsForParticipant(Participant participant) {
        StringBuilder line = new StringBuilder();
        line.append(participant.username());
        for (int i = 1 ; i <= this.totalNumberOfEvents ; i++) {
            if(participant.homework().containsKey(i) && participant.homework().get(i)) {
                line.append(",O");
            } else {
                line.append(",X");
            }
        }
        line.append(",").append(participant.getRate(this.totalNumberOfEvents));
        return line.toString();
    }

    private String cvsHeader(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("참석율");
        return header.toString();
    }
}
  • MarkdownPrinter
public class MarkdownPrinter extends StudyPrinter{
    public MarkdownPrinter(int totalNumberOfEvents, List<Participant> participants) {
        super(totalNumberOfEvents, participants);
    }

    @Override
    public void execute() throws IOException {
        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);
            });
        }
    }

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

    /**
     * | 참여자 (420) | 1주차 | 2주차 | 3주차 | 참석율 |
     * | --- | --- | --- | --- | --- |
     */
    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();
    }

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

그리고 상위클래스인 StudyPrinter 클래스와 execute 메소드를 추상화 하여 하위클래스에서 구현 할 수 있도록 변경해준다.

  • StudyPrinter
public abstract class StudyPrinter {

    protected int totalNumberOfEvents;

    protected List<Participant> participants;

    public StudyPrinter(int totalNumberOfEvents, List<Participant> participants) {
        this.totalNumberOfEvents = totalNumberOfEvents;
        this.participants = participants;
        this.participants.sort(Comparator.comparing(Participant::username));
    }

    public abstract void execute() throws IOException;

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

그러면 StudyDashboard 클래스의 print 메소드에서 어떤 클래스를 호출하냐에 따라 각기 다른 방식으로 출력이 가능하다.

private void print() throws IOException, InterruptedException {
    checkGithubIssues(getGhRepository());
    /* Console 출력 */
    new ConsolePrinter(this.totalNumberOfEvents, this.participants).execute();
    /* Csv 출력 */
    new CsvPrinter(this.totalNumberOfEvents, this.participants).execute();
    /* Markdown 출력 */
    new MarkdownPrinter(this.totalNumberOfEvents, this.participants).execute();
}

이렇게 여러 switch로 사용되는 조건문을 다형성을 이용해 복잡했던 로직을 사용하기 쉽게 분리를 한것을 확인 할 수 있다.

Related posts