[리팩토링 기술-3] 긴 매개변수를 처리하는 리팩토링

👀 매개변수가 많으면 무엇이 문제일까? 그리고 어떻게 해결할 수 있을까? 

 

 

- 어떤 함수에 매개변수가 많을수록 함수의 역할을 이해하기 어려워진다.

그렇다면 아래 내용을 생각해 봐야한다.

 

1. 과연 그 함수는 한 가지 일을 하고 있는게 맞는가?

2. 불필요한 매개변수는 없는가?

3. 하나의 레코드로 뭉칠 수 있는 매개변수 목록은 없는가?

 

 

- 어떤 매개변수를 다른 매개변수를 통해 알아낼 수 있다면,

"매개변수를 질의함수로 바꾸기(Replace Parameter with Query)"를 사용할 수 있다.

 

- 매개변수가 플래그로 사용된다면,

"플래그 인수 제거하기(Remove Flag Argument)"를 사용할 수 있다.

 

- 여러 함수가 일부 매개변수를 공통적으로 사용한다면

"여러 함수를 클래스로 묶기(Combine Function into Class)"를 통해

매개변수를 해당 클래스의 필드로 만들고

메소드에 전달해야 할 매개변수 목록을 줄일 수 있다. 

 

 

 

[ 긴 매개변수를 가진 코드를 리팩토링할 때 사용할 수 있는 기술 ] 

 

🙌  매개변수를 질의 함수로 바꾸기 (Replace Parameter With Query)

 

- 함수의 매개변수 목록은 함수의 다양성을 대변하며, 짧을수록 이해하기 좋다.

 

- 어떤 하나의 매개변수를 다른 매개변수를 통해 알아낼 수 있다면,

"중복 매개변수"라고 생각할 수 있다.

 

- 매개변수에 값을 전달하는 것은 "함수를 호출하는 쪽"의 책임이다.

가능하면 함수를 호출하는 쪽의 책임을 줄이고

함수 내부에서 책임지도록 노력한다.

 

- "임시 변수를 질의 함수로 바꾸기"와 "함수 선언 변경하기"를 통해 이 리팩토링을 적용한다.

 

 

[  Berfore  ]

 

• discountedPrice() 함수의 파라미터 discountLevel 은 다른 파라미터를 통해 값을 유추할 수 있기 때문에
• 함수로 추출하는 리팩토링을 진행한다.

 

 

[  After ] 

 

• getDiscountLevel() 이라는 함수로 추출하여 discountedPrice() 함수 내부에서 호출하여 사용한다.
• 따라서 discountedPrice() 함수의 매개변수를 줄일 수 있다.

 

 

 

---

🙌 플래그 인수 제거하기 (Remove Flag Argument)

 

- 플래그는 보통 함수에 매개변수로 전달해서

함수 내부의 로직을 분기하는데 사용한다.

 

- 플래그를 사용한 함수는 차이를 파악하기 어렵다.

 

- "조건문 분해하기(Decompose Condition)"를 활용할 수 있다.

 

 

 

[  Berfore  ]

 

• deliveryDate() 함수 내부에서 급 배송을 의미하는 isRush 에 따라 배송 날짜를 계산해주는 함수이다.
• 그런데 함수 매개변수에 플래그가 있어 함수가 많은 일을 하고 있다. → 함수를 분리하자.

 

 

 

[  After ] 

 

• 조건에 맞는 함수로 분리하여 필요한 함수를 호출해 사용하였다.

 

 

 

 

---

🙌 여러 함수를 클래스로 묶기 (Combine Function into Class)

 

- 비슷한 매개변수 목록을 여러 함수에서 사용하고 있다면

해당 메소드를 모아서 클래스를 만들 수 있다.

 

- 클래스 내부로 메소드를 옮기고 데이터를 필드로 만들면

메소드에 전달해야 하는 매개변수 목록도 줄일 수 있다.

 

 

[  Berfore  ]

• 각 함수에서 totalNumberOfEvents, participants 와 같은 비슷한 매개변수를 여러 함수에서 사용하고 있다.
• 필요한 데이터와 메서드를 옮겨 클래스로 만들어 해결할 수 있다.

public class Before_StudyDashboard {

    private final int totalNumberOfEvents;

    public Before_StudyDashboard(int totalNumberOfEvents) {
        this.totalNumberOfEvents = totalNumberOfEvents;
    }

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

    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) {
                            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(participants.size()));

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

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

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

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

 

 

[  After ] 

 

• 관련 있는 매개변수들이 여기저기 흩어져 있어 관련 있는 매개변수를 클래스로 만들었다.
  • StudyPrinter 클래스를 새로 만들어 분리했으며,
  • totalNumberOfEvents와 participants를 생성자로 주입해주었다.

• 가급적이면 관련있는 데이터를 한곳으로 옮기고 메서드를 옮기다 보면 메서드에 전달되는 매개변수를 줄일 수 있다.
  • header() 메소드의 매개변수로 보내던 participants.size()를 필드에서 받아오도록 변경했다. 

 

 

 

 

 

 

 

 

 

 

출처 : 인프런 강의 (백기선 - 코딩으로 학습하는 리팩토링)