[Refactoring] 냄새 3. 긴 함수

[tonykang22]

냄새 3. 긴 함수 (Long Function)

• 짧은 함수 vs 긴 함수

  • 함수가 길수록 더 이해하기 어렵다. vs 짧은 함수는 더 많은 문맥 전환을 필요로 한다.
  • "과거에는" 작은 함수를 사용하는 경우에 더 많은 서브루틴 호출로 인한 오버헤드가 있었다.
  • 작은 함수에 "좋은 이름"을 사용했다면 해당 함수의 코드를 보지 않고도 이해할 수 있다.
  • 어떤 코드에 "주석"을 남기고 싶다면, 주석 대신 함수를 만들고 함수의 이름으로 "의도"를 표현해보자.

• 사용할 수 있는 리팩토링 기술

  • 99%는 함수 추출하기 (Extract Function)로 해결할 수 있다.
  • 함수로 분리하면서 해당 함수로 전달해야 할 매개변수가 많아진다면 다음과 같은 리팩토링을 고려해볼 수 있다.
    • 임시 변수를 질의 함수로 바꾸기 (Replace Temp with Query)
    • 매개변수 객체 만들기 (Introduce Parameter Object)
    • 객체 통째로 넘기기 (Preserve Whole Object)
  • 조건문 분해하기 (Decompose Conditional)를 이용해 조건문을 분리할 수 있다.
  • 같은 조건으로 여러 개의 Switch 문이 있따면, 조건문을 다형성으로 바꾸기 (Replace Conditional with Polymorphism)을 사용할 수 있다.
  • 반복문 안에서 여러 작업을 하고 있어서 하나의 메소드로 추출하기 어렵다면, 반복문 쪼개기 (Split Loop)를 적용할 수 있다.

리팩토링 7. 임시 변수를 질의 함수로 바꾸기 (Replace Temp With Query)

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

예시

• 반복해서 동일한 식을 계산하는 getMarkdownForParticipant()을 먼저 추출해낼 수 있다.

• 넘어가는 인자가 3 개나 되어 유심히 관찰하니, rate 또한 반복해서 동일한 식을 계산하기에 getRate()로 추출할 수 있고, 추출한 메소드를 getMarkdownForParticipant()에서 직접 호출하게 할 수 있다.

  public class StudyDashboard {
      .
      .
      .
  private void print() throws IOException, InterruptedException {
      .
      .
      .
      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);
          });
      }
  }
  }

public class StudyDashboard {
        .
        .
        .
    private void print() throws IOException, InterruptedException {
        .
        .
        .
        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));

    }
}

리팩토링 8. 매개변수 객체 만들기 (Introduce Parameter Object)

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

예시

• record로 Participant를 묶어두었음에도 불구하고, 아래의 메소들에서 매개변수 totalNumberOfEvents와 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 {
        .
        .
        .
    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));
        }
}

• Participant와 totalNumberOfEvents를 새로운 ParticipantPrinter로 만든다.

  public class StudyDashboard {
      .
      .
      .
  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(participantPrinter);
  }
  }

• 추가 : Introduce parameter field 사용해서 반복되는 매개변수를 필드로 추출해내는 방법도 있다. (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 {
        .
        .
        .
        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 / this.totalNumberOfEvents;
        return rate;
    }

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

리팩토링 9. 객체 통째로 넘기기 (Preserve Whole Object)

• 어떤 한 레코드에서 구할 수 있는 여러 값들을 함수에 전달하는 경우, 해당 매개변수를 레코드 하나로 교체할 수 있다.
• 매개변수 목록을 줄일 수 있다. (향후에 추가할지도 모를 매개변수까지도)
• 이 기술을 적용하기 전에 의존성을 고려해야 한다.
• 어쩌면 해당 메소드의 위치가 적절하지 않을 수도 있다. (기능 편애 "Feature Envy" 냄새에 해당한다.)

예시

• 매개변수 객체가 이미 존재하는 상황에서 사용할 수 있다. (앞의 매개변수 객체 만들기에서는 이미 객체 통째로 넘기기도 사용했다.)

public class StudyDashboard {
        .
        .
        .
    private void print() throws IOException, InterruptedException {
        .
        .
        .
        participants.forEach(p -> {
                String markdownForHomework = getMarkdownForParticipant(p.username(), p.homework());
                writer.print(markdownForHomework);
            });
        }
    }

    private double getRate(Map<Integer, Boolean> homework) {
        long count = homework.values().stream()
                .filter(v -> v == true)
                .count();
        return (double) (count * 100 / this.totalNumberOfEvents);
    }

    private String getMarkdownForParticipant(String username, Map<Integer, Boolean> homework) {
        return String.format("| %s %s | %.2f%% |\n", username,
                checkMark(homework, this.totalNumberOfEvents),
                getRate(homework));
    }
}

• Participant를 받는 것이 맞는지, 변경 전의 HashMap을 받는 것이 나은지 고민이 필요하긴 하다. (다른 도메인에도 적용될 수 있는지 등에 따라)
• 과연 메소드가 현재의 클래스에 위치하는게 적절한가 / 현재의 위치가 적절한가 (Move method도 고려)

        public class StudyDashboard {
        .
        .
        .
    private void print() throws IOException, InterruptedException {
        .
        .
        .
        participants.forEach(p -> {
                String markdownForHomework = getMarkdownForParticipant(p);
                writer.print(markdownForHomework);
            });
        }
    }

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

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

리팩토링 10. 함수를 명령으로 바꾸기 (Replace Function with Command)

• 함수를 독립적인 개체인, Command로 만들어 사용할 수 있다.

• 커맨드 패턴을 적용하면 다음과 같은 장점을 취할 수 있다.

  • 부가적인 기능으로 undo 기능을 만들 수도 있다.
  • 더 복잡한 기능을 구현하는데 필요한 여러 메소드를 추가할 수 있다.
  • 상속이나 템플릿을 활용할 수도 있다.
  • 복잡합 메소드를 여러 메소드나 필드를 활용해 쪼갤 수도 있다.

• 대부분의 경우에 "커맨드"보다는 "함수"를 사용하지만, 커맨드 말고 다른 방법이 없는 경우에만 사용한다.

• 먼저 함수를 분리해보고, 그럼에도 위치가 애매하거나 추후 메소드의 복잡도가 높아질 가능성이 있을 경우에 커맨드 패턴을 통해 장점을 취한다.

• 긴 메소드를 간추리기도 좋고, 복잡도를 낮출 수 있다. (복잡한 메소드를 잘게 쪼개므로써)

• 그러나 새로운 클래스와 구조가 변경이 되어 오히려 전체적인 복잡도가 증가할 수 있다.

예제

• print() 메소드의 복잡도가 높다고 판단하여 함수 추출하기를 사용했고, 추후 확장성을 위해 StudyPrinter에 위치시킨다.

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

    private void print() throws IOException, InterruptedException {
        .
        .
        .
        // 함수 추출할 부분
        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 Participant findParticipant(String username, List<Participant> participants) {
        .
        .
        .
    }

    private String getMarkdownForParticipant(Participant p) {
        .
        .
        .
    }

    private String header(int totalNumberOfParticipants) {
        .
        .
        .
    }

    private String checkMark(Participant p, int totalEvents) {
        .
        .
        .
    }
}

• 변경 후 StudyPrinter의 로직이 조금 더 복잡해져도 적절하게 분리되어 있어 이해하기가 용이하다고 생각한다.

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

    private void print() throws IOException, InterruptedException {
        .
        .
        .
        // 변경된 부분
        new StudyPrinter(this.totalNumberOfEvents, participants).execute();
    }
    .
    .
    .
}

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)) {
            this.participants.sort(Comparator.comparing(Participant::username));

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

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

    private String getMarkdownForParticipant(Participant p) {
        .
        .
        .
    }

    private String header(int totalNumberOfParticipants) {
        .
        .
        .
    }

    private String checkMark(Participant p, int totalEvents) {
        .
        .
        .
    }
}

리팩토링 11. 조건문 분해하기 (Decompose Conditional)

• 여러 조건에 따라 달라지는 코드를 작성하다보면 종종 긴 함수가 만들어지는 것을 목격할 수 있다.
• "조건"과 "액션" 모두 의도를 표현해야한다.
• 기술적으로는 함수 추출하기와 동일한 리팩토링이지만 의도만 다르다.

예제

• 함수 추출하기를 통해 조건문에도 의도를 부여했다.

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

• 변경 후

  public class StudyDashboard {
      .
      .
      .
  private Participant findParticipant(String username, List<Participant> participants) {
      return isNewParticipant(username, participants) ?
              createNewParticipant(username, participants) :
              findExistingParticipant(username, participants);
  }
  
  private Participant findExistingParticipant(String username, List<Participant> participants) {
      return participants.stream().filter(p -> p.username().equals(username)).findFirst().orElseThrow();
  }
  
  private Participant createNewParticipant(String username, List<Participant> participants) {
      Participant participant;
      participant = new Participant(username);
      participants.add(participant);
      return participant;
  }
  
  private boolean isNewParticipant(String username, List<Participant> participants) {
      return participants.stream().noneMatch(p -> p.username().equals(username));
  }
  }

리팩토링 12. 반복문 쪼개기 (Split Loop)

• 하나의 반복문에서 여러 다른 작업을 하는 코드를 쉽게 찾아볼 수 있다. (어차피 반복문은 도니까 여러 로직을 함께 수행하는 경우가 많다.)
• 해당 반복문을 수정할 때 여러 작업을 모두 고려하여 코딩을 해야한다.
• 반복문을 여러 개로 쪼개면 보다 쉽게 이해하고 수정할 수 있다. (Bottleneck이 아니라면!)
• 성능 문제를 야기할 수 있지만, "리팩토링"은 "성능 최적화"와 별개의 작업이다. (리팩토링을 마친 이후에 성능 최적화를 시도할 수 있다.)

예제

• 반복 그 자체가 bottleneck이 아니다.

public class StudyDashboard {
        .
        .
        .
    private void print() throws IOException, InterruptedException {
        .
        .
        .
                        Date firstCreatedAt = null;
                        Participant first = null;

                        for (GHIssueComment comment : comments) {
                            // 성능 Bottleneck (Github API를 사용)
                            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;
        .
        .
        .
        }
}

• 변경 후 checkHomework(), findFirst() 로 분리하여 코드를 쉽게 이해할 수 있게 되었다. (리팩토링 != 성능 최적화)

public class StudyDashboard {
        .
        .
        .
    private void print() throws IOException, InterruptedException {
        .
        .
        .
                        checkHomework(comments, eventId);
                        firstParticipantsForEachEvent[eventId - 1] = findFirst(comments);
        .
        .
        .
        }

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

리팩토링 13. 조건문을 다형성으로 바꾸기 (Replace Conditional with Polymorphism)

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

예시

• PinterMode 에 따라 동작하는 switch 문을 다형성으로 바꾸기

public class StudyPrinter {

    private int totalNumberOfEvents;

    private List<Participant> participants;

    private PrinterMode printerMode;

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

    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를 상속받는 세 경우의 클래스를 생성했다.
  • 공통으로 사용하는 메소드는 StudyPrinter에 두고 추상 메소드인 exceute()를 생성한다.

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

public class CvsPrinter extends StudyPrinter {

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

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

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;

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