DEV ℧ Developer Diary

[Refactoring] 냄새 2. 중복 코드

19 Sep 2022

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

냄새 2. 중복 코드 (Duplicated Code)

  • 중복 코드의 단점
    • 비슷한지, 완전히 동일한 코드인지 주의 깊게 봐야한다.
    • 코드를 변경할 때, 동일한 모든 곳의 코드를 변경해야 한다.
  • 사용할 수 있는 리팩토링 기술
    • 동일한 코드를 여러 메소드에서 사용하는 경우, 함수 추출하기 (Extract Function)
    • 코드가 비슷하게 생겼지만 완전히 같지는 안은 경우, 코드 분리하기 (Slide Statements)
    • 여러 하위 클래스에 동일한 코드가 있다면, 메소드 올리기 (Pull Up Method)

예제 코드

public class StudyDashboard {

    private void printParticipants(int eventId) throws IOException {
        // Get github issue to check homework
        GitHub gitHub = GitHub.connect();
        GHRepository repository = gitHub.getRepository("dh37789/basic");
        GHIssue issue = repository.getIssue(eventId);

        // Get participants
        Set<String> participants = new HashSet<>();
        issue.getComments().forEach(c -> participants.add(c.getUserName()));

        // Print participants
        participants.forEach(System.out::println);
    }

    private void printReviewers() throws IOException {
        // Get github issue to check homework
        GitHub gitHub = GitHub.connect();
        GHRepository repository = gitHub.getRepository("dh37789/basic");
        GHIssue issue = repository.getIssue(1);

        // Get reviewers
        Set<String> reviewers = new HashSet<>();
        issue.getComments().forEach(c -> reviewers.add(c.getUserName()));

        // Print reviewers
        reviewers.forEach(System.out::println);
    }

    public static void main(String[] args) throws IOException {
        StudyDashboard studyDashboard = new StudyDashboard();
        studyDashboard.printReviewers();
        studyDashboard.printParticipants(15);

    }
}

저번 1강과 마찬가지로, Github의 이슈정보를 가져오는 API를 통해서 리팩토링을 진행하고자 한다.

리팩토링 4. 함수 추출하기

  • 함수의 분리의 기준은 여러가지 기준이 있을 수 있다. 리팩토링 책을 기준으로는 “의도”와 “구현” 분리하기를 권장 하고 있다.
  • 무슨 일을 하는 코드인지 알아내려고 노력해야 하는 코드라면 해당 코드를 함수로 분리하고 함수 이름으로 “무슨 일을 하는지” 표현할 수 있다.
  • 한줄 짜리 메소드도 괜찮은가? 의도가 잘 나타난다면 한줄 짜리도 괜찮다.
  • 거대한 함수 안에 들어있는 주석은 추출한 함수를 찾는데 있어서 좋은 단서가 될 수 있다.

위 코드의 함수중 의도는 다르지만 동일한 기능을 하는 로직이있다.

private void printParticipants(int eventId) throws IOException {
    // Get github issue to check homework
    GitHub gitHub = GitHub.connect();
    GHRepository repository = gitHub.getRepository("dh37789/basic");
    GHIssue issue = repository.getIssue(eventId);

    // Get participants
    Set<String> participants = new HashSet<>();
    issue.getComments().forEach(c -> participants.add(c.getUserName()));

    // Print participants
    participants.forEach(System.out::println);
}

private void printReviewers() throws IOException {
    // Get github issue to check homework
    GitHub gitHub = GitHub.connect();
    GHRepository repository = gitHub.getRepository("dh37789/basic");
    GHIssue issue = repository.getIssue(1);

    // Get reviewers
    Set<String> reviewers = new HashSet<>();
    issue.getComments().forEach(c -> reviewers.add(c.getUserName()));

    // Print reviewers
    reviewers.forEach(System.out::println);
}

printParticipants 함수는 매개변수로 받은 eventId의 github repository, issue 정보 습득 => 참석자 정보를 Set컬렉션에 저장 => 참석자 정보 출력의 형태로 로직이 구성되어 있고, printReviewers 함수는 함수내에 고정된 github repository, issue 정보 습득 => 리뷰어의 정보를 Set컬렉션에 저장 => 리뷰어 정보 출력의 형태로 로직이 구성되어 있다.

여기서 중복되는 코드를 정리하자면 아래와 같다.

  1. github repository, issue 정보 습득
  2. 정보를 Set컬렉션에 저장
  3. 정보 출력

해당 로직을 분리시켜 보도록 하자.

IntelliJ에서 분리하고자 하는 코드를 블럭지정하고, Ctrl + Alt + M 을 누르면, 따로 함수로 뺄 수 있다.

  • github repository, issue 정보 습득

repository와 issue 정보를 받는 부분을 getGhIssue라는 함수로 분리 시켜 준다.

private GHIssue getGhIssue(int eventId) throws IOException {
      GitHub gitHub = GitHub.connect();
      GHRepository repository = gitHub.getRepository("dh37789/basic");
      GHIssue issue = repository.getIssue(eventId);
      return issue;
}
  • 정보를 Set컬렉션에 저장

참가자 및 리뷰어의 정보를 Set 컬렉션에 저장하는 부분을 getUsernames 라는 함수로 분리시켜주자. 하지만 참가자와 리뷰어의 근본적인 속성은 issue의 username을 불러오는 것이므로 해당 getUsernames의 함수에서 이름에 대한 변수는 usernames로 명명하도록 하자.

private Set<String> getUsernames(GHIssue issue) throws IOException {
    Set<String> usernames = new HashSet<>();
    issue.getComments().forEach(c -> usernames.add(c.getUserName()));
    return usernames;
}
  • 정보 출력

비록 한줄이지만, 출력하는 함수를 분리시키면 아래와 같이 뺄수 있다.

private void print(Set<String> participants) {
    participants.forEach(System.out::println);
}
  • 리팩토링 후

주석 없이도 코드의 의도를 파악하기 쉽게 변경 되었다.

public class StudyDashboard {

  private void printParticipants(int eventId) throws IOException {
    // Get github issue to check homework
    GitHub gitHub = GitHub.connect();
    GHRepository repository = gitHub.getRepository("dh37789/basic");
    GHIssue issue = repository.getIssue(eventId);

    // Get participants
    Set<String> participants = new HashSet<>();
    issue.getComments().forEach(c -> participants.add(c.getUserName()));

    // Print participants
    participants.forEach(System.out::println);
  }

  private void printReviewers() throws IOException {
    // Get github issue to check homework
    GitHub gitHub = GitHub.connect();
    GHRepository repository = gitHub.getRepository("dh37789/basic");
    GHIssue issue = repository.getIssue(1);

    // Get reviewers
    Set<String> reviewers = new HashSet<>();
    issue.getComments().forEach(c -> reviewers.add(c.getUserName()));

    // Print reviewers
    reviewers.forEach(System.out::println);
  }

  public static void main(String[] args) throws IOException {
    StudyDashboard studyDashboard = new StudyDashboard();
    studyDashboard.printReviewers();
    studyDashboard.printParticipants(1);

  }
}

리팩토링 5. 코드 정리하기

  • 해당 리팩토링의 목적은 관련있는 코드끼리 묶어서 코드를 더 쉽게 이해할 수 있도록 한다.
  • 다른 리팩토링을 하기 위한 전처리 작용으로 주로 사용된다.
  • 함수에서 사용할 변수를 상단에 정의하기 보다는, 해당 변수를 사용하는 코드 바로 위에 선언하자.
  • 관련있는 코드끼리 묶은 다음, 함수 추출하기 (Extract Function)를 사용해서 더 깔끔하게 분리할 수도 있다.
private void printParticipants(int eventId) throws IOException {
    Set<String> participants = new HashSet<>();
    GitHub gitHub = GitHub.connect();
    GHRepository repository = gitHub.getRepository("dh37789/basic");
    GHIssue issue = repository.getIssue(eventId);
    issue.getComments().forEach(c -> participants.add(c.getUserName()));
    participants.forEach(System.out::println);
}

위의 코드에서 순서를 변경한 코드이다. Set컬렉션 변수인 participants는 앞에서 미리 선언해도 로직이 돌아가는데 크게 문제되는건 없지만, 해당 변수를 사용하는것은

GitHub gitHub = GitHub.connect();
GHRepository repository = gitHub.getRepository("dh37789/basic");
GHIssue issue = repository.getIssue(eventId);

해당 소스인 Github정보를 불러온 후에 사용된다.

위에서 말했듯이, 함수에서 사용할 변수는 사용하기전 바로 위에 선언 해주도록 하자. 그리고 서로 밀접한 기능끼리 묶어서 정리를 하면 리팩토링하기 전 코드를 쉽게 이해 할 수 있다.

코드를 정리해 보도록 하자.

private void printParticipants(int eventId) throws IOException {
    GitHub gitHub = GitHub.connect();
    GHRepository repository = gitHub.getRepository("dh37789/basic");
    GHIssue issue = repository.getIssue(eventId);

    Set<String> participants = new HashSet<>();
    issue.getComments().forEach(c -> participants.add(c.getUserName()));

    participants.forEach(System.out::println);
}

리팩토링 6. 메소드 올리기

  • 중복 코드는 당장은 잘 동작하더라도 미래에 버그를 만들어 낼 빌미를 제공한다.
    • 예) A에서 코드를 코디고, B에는 반영하지 않은 경우
  • 여러 하위 클래스에 동일한 코드가 있다면, 손쉽게 이 방법을 적용할 수 있다.
  • 비슷하지만 일부 값만 다른 경우라면, “함수 매개변수화하기” 리팩토링을 적용한 이후에, 이방법을 사용할 수 있다.
  • 하위 클래스에 있는 코드가 상위 클래스가 아닌 하위 클래스 기능에 의존하고 있다면 “필드 올리기”를 적용한 이후에 이방법을 적용할 수 있다.
  • 두 메소드가 비슷한 절차를 따르고 있다면, “템플릿 메소드 패턴”적용을 고려해 볼 수 있다.

해당 기능은 상속을 사용해서 예제가 구성되어 있어 클래스로 분리된 예제를 통해 알아보도록 하자.

  • 예제코드

  • Dashboard

public class Dashboard {

    public static void main(String[] args) throws IOException {
        ReviewerDashboard reviewerDashboard = new ReviewerDashboard();
        reviewerDashboard.printReviewers();

        ParticipantDashboard participantDashboard = new ParticipantDashboard();
        participantDashboard.printParticipants(1);
    }
}
  • ParticipantDashboard
public class ParticipantDashboard extends Dashboard {

    public void printParticipants(int eventId) throws IOException {
        // Get github issue to check homework
        GitHub gitHub = GitHub.connect();
        GHRepository repository = gitHub.getRepository("dh37789/basic");
        GHIssue issue = repository.getIssue(eventId);

        // Get participants
        Set<String> participants = new HashSet<>();
        issue.getComments().forEach(c -> participants.add(c.getUserName()));

        // Print participants
        participants.forEach(System.out::println);
    }

}
  • ReviewerDashboard
public class ReviewerDashboard extends Dashboard {

    public void printReviewers() throws IOException {
        // Get github issue to check homework
        Set<String> reviewers = new HashSet<>();
        GitHub gitHub = GitHub.connect();
        GHRepository repository = gitHub.getRepository("dh37789/basic");
        GHIssue issue = repository.getIssue(1);

        // Get reviewers
        issue.getComments().forEach(c -> reviewers.add(c.getUserName()));

        // Print reviewers
        reviewers.forEach(System.out::println);
    }

}

Dashboard 는 각각 ParticipantDashboard와 ReviewerDashboard를 인스턴스화 하여 내부 함수를 호출하는 개념으로 이루어져 있다.

먼저 ParticipantDashboard와 ReviewerDashboard는 거의 동일하다, 둘의 중복코드를 하나로 합쳐주기 위해서 매개변수를 받는 쪽에 맞춰 ReviewerDashboard 클래스의 printReviewers() 메서드에 매개변수를 추가해 줄것이다.

GHIssue issue = repository.getIssue(1);

issue의 번호를 매개변수로 받아서 ParticipantDashboard 클래스와 맞춰주도록 하자.

GHIssue issue = repository.getIssue(eventId);

Dashboard 클래스에서 호출하고 있는, printReviewers()의 호출에 맞춰주기 위해 1의 매개변수를 갖는 printUsernames(int eventId)를 호출한다.

public class ReviewerDashboard extends Dashboard {

    public void printReviewers() throws IOException {
        printUsernames(1);
    }

    private void printUsernames(int eventId) throws IOException {
        // Get github issue to check homework
        GitHub gitHub = GitHub.connect();
        GHRepository repository = gitHub.getRepository("dh37789/basic");
        GHIssue issue = repository.getIssue(eventId);

        // Get reviewers
        Set<String> reviewers = new HashSet<>();
        issue.getComments().forEach(c -> reviewers.add(c.getUserName()));

        // Print reviewers
        reviewers.forEach(System.out::println);
    }
}

이렇게 하면 기존의 Dashboard의 영향없이 동일한 로직을 호출 할 수 있다.

하지만 printUsernames에 eventId의 매개변수를 추가함으로써 ParticipantDashboard 클래스의 printParticipants 함수와 동일해졌다. 만약 리뷰의 정보를 가져오는 로직이 변경될 경우 두클래스 모두 두번의 수정을 해줘야 한다. 이러한 문제점을 없애주기 위해 부모클래스인 Dashboard 클래스에 중복된 코드를 넣어 사용하도록 하자.

  • Dashboard

중복되는 부분을 부모클래스인 Dashboard 클래스에 넣어, 한번만 수정해도 모든 클래스에 적용 될 수 있도록 하였다.

public class Dashboard {

    public static void main(String[] args) throws IOException {
        ReviewerDashboard reviewerDashboard = new ReviewerDashboard();
        reviewerDashboard.printReviewers();

        ParticipantDashboard participantDashboard = new ParticipantDashboard();
        participantDashboard.printUsernames(1);
    }

    public void printUsernames(int eventId) throws IOException {
        // Get github issue to check homework
        GitHub gitHub = GitHub.connect();
        GHRepository repository = gitHub.getRepository("dh37789/basic");
        GHIssue issue = repository.getIssue(eventId);

        // Get participants
        Set<String> participants = new HashSet<>();
        issue.getComments().forEach(c -> participants.add(c.getUserName()));

        // Print participants
        participants.forEach(System.out::println);
    }
}
  • ParticipantDashboard
public class ParticipantDashboard extends Dashboard {

}
  • ReviewerDashboard
public class ReviewerDashboard extends Dashboard {

  public void printReviewers() throws IOException {
    super.printUsernames(1);
  }

}

Related posts