긴 함수(Long Function)

박상훈·2022년 8월 9일
0
함수가 길 수록 더 이해하기 어려움
과거에 작은 함수를 사용하는 경우 더 많은 서브루틴 호출로 인한 오버헤드 발생
작은 함수에 좋은 이름을 사용한다면 함수의 코드를 보지 않고 이해 가능
어떤 코드에 주석을 남기고 싶으면 대신 함수를 만들고 함수의 이름으로 의도 표현

임시 변수를 질의 함수로 바꾸기(Replace Temp with Query)

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

매개변수 객체 만들기(Introduce Parameter Object)

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

객체 통째로 넘기기(Preserve Whole Object)

  • 어떤 한 레코드에서 구할 수 있는 여러 값들을 함수에 전달하는 경우 매개변수를 레코드로 교체
  • 매개변수 목록 줄일 수 있다, 더 늘어날 매개변수를 미연에 방지
  • 의존성을 고려하여 적용
  • 메서드의 위치가 적절하지 않을 수 있다
    • 객체를 넘겼을 때 기능 편애 냄새에 해당되는 경우(자기 모듈 보다 다른 모듈에 상호작용이 잦을 때)

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

  • 함수를 독립적인 객체인 Command로 만들어 사용할 수 있다
  • 커맨드 패턴 장점
    • undo 기능 만들기
    • 복잡한 기능을 구현하는데 필요한 여러 메서드를 추가할 수 있다
    • 상속이나 템플릿 활용 가능
    • 복잡한 메서드를 여러 메서드나 필드를 활용해 쪼갤 수 있다
  • 대부분의 경우 커맨드 보다 함수를 사용하며 커맨드 말고 다른방법이 없는 경우 사용

조건문 분해하기(Decompose Conditional)

  • 여러 조건에 따라 달라지는 코드를 작성하다보면 종종 긴 함수가 만들어진다
  • 조건 과 액션 모두 의도를 표현해야 한다
  • 기술적으로 함수 추출하기와 동일하지만 의도만 다름

분기점에서 어떤 행위를 하는지 한 줄씩 읽어가며 전체 코드를 이해해야 한다
조건에 따라 분해하여 분기내용, true, false 3줄로 어떤 행위가 발생할지 내부코드를 보지 않고 파악이 가능하다

before

class Refactoring {
    private Participant findParticipant(String username, List<Participant> participants) {
        Participant participant = null;
        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;
    }
}

after

class Refactoring {
  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) {
    Participant participant;
    participant = participants.stream().filter(p -> p.username().equals(username))
            .findFirst()
            .orElseThrow();
    return participant;
  }

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

반복문 쪼개기(Split Loop)

  • 하나의 반복문에서 여러 작업의 코드를 쉽게 찾아볼 수 있다
  • 반복문을 수정할 때 여러 작업을 모두 고려하여 코딩 해야한다
  • 반복문을 쪼개면 쉽게 이해하고 수정 가능하다
  • 성능 문제(리팩토링 / 성능 최적화)는 별개의 작업이며 리팩토링 후 성능 최적화 시도 가능

반복문을 작업 단위로 쪼개는 경우 작업 개수만큼 반복문을 생성하여 작업 코드를 나누어 메서드를 생성한다
위 4번째에서 언급했듯이 성능 문제가 생길 수 있으나 리팩토링 후 성능 최적화 시도로 해결

before

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

after

class Refactoring {
    private void print() throws IOException, InterruptedException {
        GHRepository ghRepository = getGhRepository();
        checkGithubIssues(ghRepository);
        new StudyPrint(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(() -> {
                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;
    }
}

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

  • 여러 타입에 따라 각기 다른 로직으로 처리해야 하는 경우 다형성을 적용해 조건문을 보다 명확하게 분리 가능
  • 반복되는 switch 문을 각기 다른 클래스를 만들어 제거 가능
  • 공통으로 사용되는 로직은 상위클래스에 두며 달라지는 부분만 하위클래스에 두어 변경점에 강조

enum 타입별 하위 클래스를 생성하고 상위 클래스를 상속받아 execute 를
오버라이딩 하고 switch 문 작업을 타입별로 쪼갠다
이 후 불필요한 PrinterMode enum 클래스 제거
변경 후checkMark 메서드는 하위 클래스에 2곳에서 사용중이므로 부모 클래스에 위치하고
하위클래스에서 슈퍼클래스 메서드에 접근하는 구조로 중복을 제거
Use 클래스를 보면 StudyPrinter 클래스 생성자에 type 을 넘겨 생성하던 방식에서
타입을 제거하고 필요한 하위클래스를 직접 생성하는 구조로 변경

before

public enum PrinterMode {
    CONSOLE, CVS, MARKDOWN
}

class Refactoring {
    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);
                    });
                }
            }
        }
    }
}

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

after

abstract class Refactoring {
    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();
    }
}

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

    // ... 오버라이딩한 메서드에서 필요한 메서드들 상위클래스에서 빼낸다
}

class Use {
    private void print() throws IOException, InterruptedException {
        checkGithubIssues(getGhRepository());
        new MarkdownPrinter(this.totalNumberOfEvents, this.participants).execute();
    }
}
profile
엔지니어

0개의 댓글