DEV ℧ Developer Diary

[Refactoring] 냄새 8. 산탄총 수술

13 Dec 2022

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

냄새 8. 산탄총 수술

  • 어떤 한 변경 사항이 생겼을 때 여러 모듈을 (여러 함수 또는 여러 클래스를) 수정해야 하는 상황
    • “뒤엉킨 변경” 냄새와 유사하지만 반대의 상황이다.
    • 예) 새로운 결제 방식을 도입하려면 여러 클래스의 코드를 수정해야 한다.
  • 변경 사항이 여러곳에 흩어진다면 찾아서 고치기도 어렵고 중요한 변경 사항을 놓칠 수 있는 가능성도 생긴다.
  • 관련 리팩토링 기술
    • “함수 옮기기 (Move Function)” 또는 “필드 옮기기 (Move Field)”를 사용해서 필요한 변경 내역을 하나의 클래스로 모을 수 있다.
    • 비슷한 데이터를 사용하는 여러 함수가 있다면 “여러 함수를 클래스로 묶기 (Combine Functions into Class)”를 사용할 수 있다.
    • “단계 쪼개기 (Split Phase)”를 사용해 공통으로 사용되는 함수의 결과물들을 하나로 묶을 수 있다.
    • “함수 인라인 (Inline Function)” 과 “클래스 인라인(Inline Class)” 로 흩어진 로직을 한 곳으로 모을 수 있다.

리팩토링 27. 필드 옮기기

  • 좋은 데이터 구조를 가지고 있다면, 해당 데이터에 기반한 어떤 행위를 코드로 (메소드나 함수) 옮기는 것도 간편하고 단순해진다.
  • 처음에는 타당해 보였던 설계적인 의사 결정도 프로그램이 다루고 있는 도메인과 데이터 구조에 대해 더많이 익혀나가면서, 틀린 의사 결정으로 바뀌는 경우도 있다.
  • 필드를 옮기는 단서:
    • 어떤 데이터를 항상 어떤 레코드와 함께 전달하는 경우.
    • 어떤 레코드를 변경할 때 다른 레코드에 있는 필드를 변경해야 하는 경우.
    • 여러 레코드에 동일한 필드를 수정해야 하는 경우
    • (여기서 언급한 ‘레코드’는 클래스 또는 객체로 대체할 수도 있음)
  • 필드를 옮겨 하나로 만들 수 있다면, 여러곳을 수정하는것이 아닌 한곳만 수정해서 로직을 변경 할 수 있을 것이다.

아래는 해당 리팩토링의 예시이다.

  • Customer
public class Customer {

    private String name;

    private double discountRate;

    private CustomerContract contract;

    public Customer(String name, double discountRate) {
        this.name = name;
        this.discountRate = discountRate;
        this.contract = new CustomerContract(dateToday());
    }

    public double getDiscountRate() {
        return discountRate;
    }

    public void becomePreferred() {
        this.discountRate += 0.03;
        // 다른 작업들
    }

    public double applyDiscount(double amount) {
        BigDecimal value = BigDecimal.valueOf(amount);
        return value.subtract(value.multiply(BigDecimal.valueOf(this.discountRate))).doubleValue();
    }

    private LocalDate dateToday() {
        return LocalDate.now();
    }
}
  • CustomerContract
public class CustomerContract {

    private LocalDate startDate;

    public CustomerContract(LocalDate startDate) {
        this.startDate = startDate;
    }
}

먼저 해당 코드에 대한 테스트를 돌려 로직이 정상적으로 수행하는지 확인해보자.

class CustomerTest {
    @Test
    void applyDiscount() {
        Customer customer = new Customer("keesun", 0.5);
        assertEquals(50, customer.applyDiscount(100));

        customer.becomePreferred();
        assertEquals(47, customer.applyDiscount(100));
    }
}

테스트를 실행하면 정상적으로 돌아가는 것을 확인 할 수 있다.

리팩토링001

처음으로 살펴볼 곳은 Customer 클래스이다. 아래의 메소드의 becomePreferred 와 applyDiscount는 this를 통해 직접적으로 전역 변수에 접근을 하고 있다.

public void becomePreferred() {
    this.discountRate += 0.03;
    // 다른 작업들
}

public double applyDiscount(double amount) {
    BigDecimal value = BigDecimal.valueOf(amount);
    return value.subtract(value.multiply(BigDecimal.valueOf(this.discountRate))).doubleValue();
}

해당 필드를 getter와 setter로 빼서 캡슐화를 진행 해 볼 수 있다.

public void becomePreferred() {
    this.setDiscountRate(this.getDiscountRate() + 0.03);
    // 다른 작업들
}

public double applyDiscount(double amount) {
    BigDecimal value = BigDecimal.valueOf(amount);
    return value.subtract(value.multiply(BigDecimal.valueOf(this.getDiscountRate()))).doubleValue();
}

하지만 여기서 discountRate 필드를 CustomerContract 해당 클래스로 필드를 옮겨 공통적으로 관리를 할 수 도 있다.

먼저 CustomerContract의 클래스에 전역 필드를 생성 해주고 생성자에 추가를 해주자. 이후 getter, setter를 만들어 해당 필드에 접근 할 수있도록 만들어 준다.

public class CustomerContract {

    private LocalDate startDate;

    private double discountRate;

    public CustomerContract(LocalDate startDate, double discountRate) {
        this.startDate = startDate;
        this.discountRate = discountRate;
    }

    public double getDiscountRate() {
      return discountRate;
    }

    public void setDiscountRate(double discountRate) {
      this.discountRate = discountRate;
    }
}

이후 Customer 클래스에서는 해당 필드를 삭제한뒤 CustomerContract 의 변수인 contract에서 해당 필드의 정보를 가져 올 수 있도록 변경할 수 있다.

public class Customer {

    private String name;

    private CustomerContract contract;

    public Customer(String name, double discountRate) {
        this.name = name;
        this.contract = new CustomerContract(dateToday(), discountRate);
    }

    public void setDiscountRate(double discountRate) {
        this.contract.setDiscountRate(discountRate + 0.03);
    }

    public double getDiscountRate() {
        return this.contract.getDiscountRate();
    }

    public void becomePreferred() {
        this.setDiscountRate(this.getDiscountRate() + 0.03);
        // 다른 작업들
    }

    public double applyDiscount(double amount) {
        BigDecimal value = BigDecimal.valueOf(amount);
        return value.subtract(value.multiply(BigDecimal.valueOf(this.getDiscountRate()))).doubleValue();
    }

    private LocalDate dateToday() {
        return LocalDate.now();
    }
}

해당 작업 이후 더 나아가, getDiscountRate, setDiscountRate의 작업을 Customer에서 없애고 직접적으로 호출하도록 변경을 할 수 있을것이다, 또한 applyDiscount 와 같은 작업은 나중에 서비스의 크기가 커질때 CustomerContract의 클래스에서 작업이 필요한지에 대한 고민이 추가적으로 들어갈 수도 있다.

리팩토링 28. 함수 인라인

  • “함수 추출하기 (Extract Function)”의 반대에 해당하는 리팩토링
    • 함수로 추출하여 함수 이름으로 의도를 표현하는 방법
  • 간혹, 함수 본문이 함수 이름 만큼 또는 그보다 더 잘 의도를 표현하는 경우도 있다.
  • 함수 리팩토링이 잘못된 경우에 여러 함수를 인라인하여 커다란 함수를 만든 다음에 다시 함수 추출하기를 시도할 수 있다.
  • 단순히 메소드 호출을 감싸는 우회형 (indirection) 메소드라면 인라인으로 없앨 수 있다.
  • 상속 구조에서 오버라이딩 하고 있는 메소드는 인라인 할 수 없다. (해당 메소드는 일종의 규약 이니까)

예제를 살펴보도록 하자.

  • Driver
public class Driver {

    private int numberOfLateDeliveries;

    public Driver(int numberOfLateDeliveries) {
        this.numberOfLateDeliveries = numberOfLateDeliveries;
    }

    public int getNumberOfLateDeliveries() {
        return this.numberOfLateDeliveries;
    }
}
  • Rating
public class Rating {

    public int rating(Driver driver) {
        return moreThanFiveLateDeliveries(driver) ? 2 : 1;
    }

    private boolean moreThanFiveLateDeliveries(Driver driver) {
        return driver.getNumberOfLateDeliveries() > 5;
    }
}

해당 리팩토링은 간단하다. moreThanFiveLateDeliveries으로 5회 이상인지 조건을 검증하는 메소드는 따로 메소드로 추출함으로써 크게 의미가 변하는 메소드가 아니기 때문에 rating 메소드에서 직접 비교해 주어도 무방하다. moreThanFiveLateDeliveries 의 내부로직을 rating 에 넣어준다.

public class Rating {

    public int rating(Driver driver) {
        return driver.getNumberOfLateDeliveries() > 5 ? 2 : 1;
    }
}

리팩토링 29. 클래스 인라인

  • “클래스 추출하기 (Extract Class)”의 반대에 해당하는 리팩토링
  • 리팩토링 하는 중에 클래스의 책임을 옮기다보면 클래스의 존재 이유가 빈약해지는 경우가 발생할 수 있다.
  • 두개의 클래스를 여러 클래스로 나누는 리팩토링을 하는 경우에, 우선 “클래스 인라인”을 적용해서 두 클래스의 코드를 한 곳으로 모으고 그런 다음에 “클래스 추출하기”를 적용해서 새롭게 분리하는 리팩토링을 적용할 수 있다.

간단한 예제를 살펴보자.

  • Shipment
public class Shipment {

    private TrackingInformation trackingInformation;

    public Shipment(TrackingInformation trackingInformation) {
        this.trackingInformation = trackingInformation;
    }

    public TrackingInformation getTrackingInformation() {
        return trackingInformation;
    }

    public void setTrackingInformation(TrackingInformation trackingInformation) {
        this.trackingInformation = trackingInformation;
    }

    public String getTrackingInfo() {
        return this.trackingInformation.display();
    }
}
  • TrackingInformation
public class TrackingInformation {

    private String shippingCompany;

    private String trackingNumber;

    public TrackingInformation(String shippingCompany, String trackingNumber) {
        this.shippingCompany = shippingCompany;
        this.trackingNumber = trackingNumber;
    }

    public String display() {
        return this.shippingCompany + ": " + this.trackingNumber;
    }

    public String getShippingCompany() {
        return shippingCompany;
    }

    public void setShippingCompany(String shippingCompany) {
        this.shippingCompany = shippingCompany;
    }

    public String getTrackingNumber() {
        return trackingNumber;
    }

    public void setTrackingNumber(String trackingNumber) {
        this.trackingNumber = trackingNumber;
    }
}

해당 예제는 TrackingInformation 에서 배송 관련 정보를 조회해주는 간단한 예제이다.

만약 TrackingInformation의 클래스에서 shippingCompany, trackingNumber의 필드를 Shipment의 클래스에 클래스 인라인을 통해 하나로 옮겨주도록 하자.

먼저 필드를 Shipment로 옮겨주고 생성자를 만들어준다.

public class Shipment {

    private String shippingCompany;

    private String trackingNumber;

    private TrackingInformation trackingInformation;

    public Shipment(String shippingCompany, String trackingNumber) {
        this.shippingCompany = shippingCompany;
        this.trackingNumber = trackingNumber;
    }

    public Shipment(TrackingInformation trackingInformation) {
        this.trackingInformation = trackingInformation;
    }

    public TrackingInformation getTrackingInformation() {
        return trackingInformation;
    }

    public void setTrackingInformation(TrackingInformation trackingInformation) {
        this.trackingInformation = trackingInformation;
    }

    public String getTrackingInfo() {
        return this.trackingInformation.display();
    }
}

이후 TrackingInformation의 클래스에서 해당 필드를 사용하는 메소드를 모두 Shipment로 옮겨 주도록 하자. 해당 메소드를 그대로 복사하는 이유는 먼저 컴파일 에러를 줄일 수 있고, 더쉽다. 어자피 TrackingInformation가 삭제될 것인데 삭제되기 전에 컴파일에러를 고치기 위해 코드를 수정하고, TrackingInformation를 삭제한뒤 또 수정하는것은 비효율적으로 느껴지기 때문이다. 물론 정답은 없다.

public class Shipment {

    private String shippingCompany;

    private String trackingNumber;

    private TrackingInformation trackingInformation;

    public Shipment(String shippingCompany, String trackingNumber) {
        this.shippingCompany = shippingCompany;
        this.trackingNumber = trackingNumber;
    }

    public Shipment(TrackingInformation trackingInformation) {
        this.trackingInformation = trackingInformation;
    }

    public TrackingInformation getTrackingInformation() {
        return trackingInformation;
    }

    public void setTrackingInformation(TrackingInformation trackingInformation) {
        this.trackingInformation = trackingInformation;
    }

    public String getTrackingInfo() {
        return this.trackingInformation.display();
    }

    public String display() {
        return this.shippingCompany + ": " + this.trackingNumber;
    }

    public String getShippingCompany() {
        return shippingCompany;
    }

    public void setShippingCompany(String shippingCompany) {
        this.shippingCompany = shippingCompany;
    }

    public String getTrackingNumber() {
        return trackingNumber;
    }

    public void setTrackingNumber(String trackingNumber) {
        this.trackingNumber = trackingNumber;
    }
}

이후 TrackingInformation의 정보들을 삭제해 주도록 하자. 이렇게 하면 깔끔하게, Shipment 클래스 인라인을 수행해 줄 수 있다.

public class Shipment {

    private String shippingCompany;

    private String trackingNumber;

    public Shipment(String shippingCompany, String trackingNumber) {
        this.shippingCompany = shippingCompany;
        this.trackingNumber = trackingNumber;
    }

    public String getTrackingInfo() {
        return this.display();
    }

    public String display() {
        return this.shippingCompany + ": " + this.trackingNumber;
    }

    public String getShippingCompany() {
        return shippingCompany;
    }

    public void setShippingCompany(String shippingCompany) {
        this.shippingCompany = shippingCompany;
    }

    public String getTrackingNumber() {
        return trackingNumber;
    }

    public void setTrackingNumber(String trackingNumber) {
        this.trackingNumber = trackingNumber;
    }
}

Related posts