DEV ℧ Developer Diary

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

20 Sep 2022

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

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

리팩토링 9. 객체 통째로 넘기기

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

예제코드

먼저 매개변수가 여럿 넘어가는것중에 같은 객체를 사용하는지 확인해보자.

  • Participant
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);
    }
}
  • StudyDashboard
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.username(), p.homework());
        writer.print(markdownForHomework);
    });
}

...

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

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

Participant record를 보면 username과 homework의 필드를 가지고 있는것을 볼 수가 있는데, StudyDashboard 클래스의 부분을 보면 getRate, getMarkdownForParticipant, checkMark 의 함수에서 participants에서 추출한 필드와 동일한 homework나 username 필드를 사용하는 것을 볼 수 있다.

해당 부분을 각각의 파라미터를 넘겨주는것이 아니라 매개변수를 줄여주기 위해 participants 전부를 넘겨주는걸로 수정 해줄 수 있다.

이렇게 되면 Participant에 필드가 추가되거나 함수에 추가적으로 파라미터가 필요할 경우 큰 변화없이 데이터를 추가할 수 있다.

  • 변경후
        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);
            });
        }

    ...

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

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

하지만 여기서 다시한번 고민을 해주는게 좋다.

과연 getRate, getMarkdownForParticipant, checkMark 해당 함수들이 Participant에 의존 하는것이 맞는가. 만약 getRate을 다른곳에서도 사용할 수 있다면?

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

기존과 동일하게 HashMap을 매개변수로 받아 사용 할 수 있도록 선택하는것도 중요하다.

즉, 무조건 객체로 만들어서 매개변수를 줄인다고 좋은 코드가 되는것이 아니다. 재사용성을 고려해야 한다. getRate 를 참가자(Participant)의 비율을 계산한다.의 의도에 맞게 Participant 쪽으로 옮겨주도록 하자.

함수명을 블럭하고 F6을 누르면 매개변수에 해당하는 객체로 메소드를 옮길 수 있다. 다만, 옮겨지면서 의존성이 변경된다.

이제는 StudyDashboard를 의존하는 getRate 함수로 변경되었다. 여기서 StudyDashboard를 의존하는 것은 totalNumberOfEvents 하나뿐이므로, 해당 변수를 매개변수로 받아서 의존성을 줄여 공통적으로 쓰일 수 있도록 하자.

  • Participant
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(StudyDashboard studyDashboard) {
        long count = homework().values().stream()
                .filter(v -> v == true)
                .count();
        return (double) (count * 100 / studyDashboard.totalNumberOfEvents);
    }
}

StudyDashboard의 의존성을 없애고, StudyDashboard에서 gerRate를 호출하는 부분을 변경해 주었다.

  • Participant
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);
    }
}
  • StudyDashboard
private String getMarkdownForParticipant(Participant participant) {
    return String.format("| %s %s | %.2f%% |\n", participant.username(),
            checkMark(participant, this.totalNumberOfEvents),
            participant.getRate(this.totalNumberOfEvents));
}

이렇게 과연 이 메소드가 클래스에 의존하는것이 맞는지, 이 메소드가 지금 위치해 있는곳이 맞는가. 다른곳으로 옮기면 재사용성이 늘어날 수 있지 않나. 충분한 고민이 필요하다.

리팩토링 10. 함수를 명령으로 바꾸기

  • 함수를 독립적인 객체인, Command로 만들어 사용할 수 있다.
  • 커맨드 패턴을 적용하면 다음과 같은 장점을 취할 수 있다.
    • 부가적인 기능으로 undo 기능을 만들 수도 있다.
    • 더 복잡한 기능을 구현하는데 필요한 여러 메소드를 추가할 수 있다.
    • 상속이나 템플릿을 활용할 수도 있다.
    • 복잡한 메소드를 여래 메소드나 필드를 활용해 쪼갤 수도 있다.
  • 대부분의 경우에 “커맨드” 보다는 “함수”를 사용하지만, 커맨드 말고 다른 방법이 없는 경우에만 사용한다.

커맨더 패턴은 추후 공부해서 추가하도록 하자!

예제코드

해당 부분은 참가자의 객체를 받아 해당 정보를 Markdown 파일로 출력하는 파일이다. 추후에 CSV파일이나 엑셀 또는 콘솔창에 출력하도록 변경된다면 많은 복잡도가 예상되므로, Command 로추출해보도록 하자

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

이후 StudyPrinter라는 Class(또는 interface)를 생성해서 해당 함수를 옮겨준다.

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

그렇다면 기존에 사용하던 markdownForHomework, header 와 같은 Markdown을 출력할때 실행하는 함수는 StudyDashboard에서 필요가 없어지게된다. 추출한 함수인 execute에서 사용하는 Markdown 출력함수들을 모두 StudyPrinter로 옮겨주자.

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

    /**
     * | 참여자 (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();
    }

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

하지만 StudyPrinter 에서는 Participant나 totalNumberOfEvents의 필드를 받는 곳이 없으므로, 전역변수로 추가해준뒤 생성자로 해당 데이터를 받도록 하자. 또한 execute 같은 함수는 매개변수로 받던 것들을 지워주고 전역변수에서 받아올 수 있도록 수정해준다.

  • StudyPrinter
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 participants) {
        return String.format("| %s %s | %.2f%% |\n", participants.username(), checkMark(participants, this.totalNumberOfEvents),
                participants.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();
    }

    /**
     * |: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문이 존재하던 곳은 아래와 같이 호출해 줄 수 있을 것이다.

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

추후에 markdown 파일명을 변경한다던가, 또는 Studyprinter를 CsvPrinter, ExcelPrinter 와 같이 interface로 만들어 구현하면 CSV파일이나 엑셀 또는 콘솔 파일로 출력하는 로직을 따로 분리해서 구현할 수 있게된다.

리팩토링 11. 조건문 분해하기

  • 여러 조건에 따라 달라지는 코드를 작성하다보면 종종 긴 함수가 만들어지는 것을 목격할 수 있다.
    • if ~ else 또는 if - true, if - false 가 해야할 일에 대해 작성하다 보면 코드가 길어질 수 있다.
  • “조건”과 “액션” 모두 “의도”를 표현해야한다.
  • 기술적으로는 “함수 추출하기”와 동일한 리팩토링이지만 의도만 다를 뿐이다.

예제코드

해당 함수를 살펴보면 if문을 포함한 내부 로직이 눈에 잘 들어오지 않는다. 가독성이 좋도록 리팩토링을 진행해보자.

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

각각의 조건문 또는 조건에 따라 행해지는 행위를 분리시킴으로써 어떤 한눈에 봐도 이해할 수 있도록 함수를 분리했다.

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

    return participant;
}

private boolean isNewParticipant(String username, List<Participant> participants) {
    return participants.stream().noneMatch(p -> p.username().equals(username));
}

private Participant createNewPariticipants(String username, List<Participant> participants) {
    Participant participant;
    participant = new Participant(username);
    participants.add(participant);
    return participant;
}

private Participant findExistingParticipant(String username, List<Participant> participants) {
    Participant participant;
    participant = participants.stream().filter(p -> p.username().equals(username)).findFirst().orElseThrow();
    return participant;
}

또한 해당 if ~ else의 함수의 경우 삼항연산자를 이용해 소스를 더 간추려 줄 수 있다.

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

또한 여기서 진행하지는 않지만, isNewParticipant, createNewPariticipants, findExistingParticipant 각각에 들어가는 매개변수들이 중복되는 것을 볼 수있다. 해당 매개변수를 클래스로 분리해서 주는것도 좋은 방법이 될것이다.

Related posts