Shotgun Surgery

 

산탄총 수술이라는 뜻의 해당 리팩터링 냄새는 애플리케이션의 변경 사항이 생겼을 때 

여러 모듈을 수정해야 하는 경우를 빗대어 표현한 것이다.

 

이는 코드의 응집도가 낮고 결합도 높다는 것을 의미한다. 

 

변경 사항이 여러곳에 흩어진다면 찾아서 고치기도 어렵고 중요한 변경 사항을 놓칠 수 있는 가능성도 생긴다.

 

관련된 기술로는 다음과 같다.

• 함수 옮기기 또는 필드 옮기기로 필요한 변경 내역을 하나의 클래스로 모을 수 있다.
• 비슷한 데이터를 사용하는 여러 함수가 있다면 여러 함수를 클래스로 묶을 수도 있다.
• 단계 쪼개기를 사용해 공통으로 사용되는 함수의 결과물들을 하나로 묶을 수 있다.

 

 

Move Field

 

좋은 데이터 구조를 가지고 있다면, 해당 데이터에 기반한 어떤 행위를 코드로 옮기는 것도 간편하고 단순해진다.

 

필드를 옮기는 단서

 

1. 어떤 데이터를 항상 어떤 레코드와 함께 전달하는 경우

2. 어떤 레코드를 변경할 때 다른 레코드에 있는 필드를 변경해야 하는 경우

3. 여러 레코드에 동일한 필드를 수정해야 하는 경우

 

레코드는 클래스나 객체로 대체할 수 있다.

 

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

 

다음 클래스에서 discountRate가 추후 CustomerContract 클래스로 이동되어야 맞다고 생각하고 옮겨보자.

 

이 경우  discountRate에 직접 접근하는 필드를 전부 getter, setter를 사용하도록 사전작업을 해주어야 한다.

 

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 double getDiscountRate() {
        return this.contract.getDiscountRate();
    }

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

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

 

그래야 해당 필드를 지웠을 때 getter , setter 사용하는 코드를 고칠 필요가 없어진다.

 

더 나아가면 여기서 해당 필드를 사용하는 메서드들이 Customer 클래스에서 하는 것이 아닌 CustomerContract에서 하는 일이 맞지 않는지 옮기는 것도 고려할 수 있습니다.

 

 

Inline Function

 

함수 추출하기의 반대에 해당하는 리팩터링이다.

 

간혹 함수 본문이 함수 이름만큼 또는 그보다 더 잘 의도를 표현하는 경우도 있다. 

 

함수 리팩터링이 잘못된 경우에 여러 함수를 인라인 하여 커다란 함수를 만든 다음에 다시 함수 추출하기를 시도할 수 있다. 

또한 단순히 메서드 호출을 감싸는 indirection 메서드라면 인라인으로 없앨 수 있다.

 

 

Inline Class

 

이는 리팩터링을 하는 중에 클래스의 책임을 옮기다 보면 클래스의 존재 이유가 빈약해지는 경우가 발생할 수 있다.

두 개의 클래스를 여러 클래스로 나누는 리팩터링을 하는 경우에, 우선 클래스 인라인을 적용해서 두 클래스의 코드를 한 곳으로 모으고 그런 다음에 클래스 추출하기를 적용해서 새롭게 분리하는 리팩터링을 적용할 수 있다.

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

 

Shipment Class에서 TrackingInformation를 사용하고 있다.

 

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

 

이를 단순 위임하는 클래스로 보이기 때문에 이를 인라인 클래스를 적용해 Shipment Class으로 옮겨보자.

 

이때 메서드를 먼저 옮기는지, 필드를 먼저 옮기는지는 선택이다. 

 

다만 컴파일 오류를 줄이기 위해서 필드만 먼저 옮긴이후 getter/setter를 만든 이 후 메서드를 옮겨보자.

 

public class Shipment {

    private String shippingCompany;

    private String trackingNumber;

    public Shipment(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;
    }
}

 

이후 해당하는 테스트 코드에서 에러가 발생한다. 테스트 코드를 고쳐보자.

 

as

@Test
void trackingInfo() {
    Shipment shipment = new Shipment("UPS", "12345");
    assertEquals("UPS: 12345", shipment.getTrackingInfo());
}

 

getTrackingInfo()를 클라이언트에서 사용하니 display의 메서드 이름을 getTrackingInfo()로 변경하여 처리한다.

public class Shipment {

    private String shippingCompany;

    private String trackingNumber;

    public Shipment(String shippingCompany, String trackingNumber) {
        this.shippingCompany = shippingCompany;
        this.trackingNumber = 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;
    }
    public String getTrackingInfo() {
        return this.shippingCompany + ": " + this.trackingNumber;
    }
}