DEV ℧ Developer Diary

[Refactoring] 냄새 6. 가변 데이터 (2)

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

냄새 6. 가변 데이터 (2)

리팩토링 21. 파생 변수를 질의 함수로 바꾸기

  • 변경할 수 있는 데이터를 최대한 줄이도록 노력해야 한다.
  • 계산해서 알아낼 수 있는 변수는 제거할 수 있다.
    • 계산 자체가 데이터의 의미를 잘 표현하는 경우도 있다.
    • 해당 변수가 어디선가 잘못된 값으로 수정될 수 있는 가능성을 제거할 수 있다.
  • 계산에 필요한 데이터가 변하지 않는 값이라면, 계산 결과에 해당하는 데이터 역시 불변 데이터기 때문에 해당 변수는 그대로 유지할 수 있다.

예시를 살펴보자, 해당 코드는 Discount 즉, 할인을 계산해서 반환하는 클래스이다.

public class Discount {
    private double discountedTotal;
    private double discount;

    private double baseTotal;

    public Discount(double baseTotal) {
        this.baseTotal = baseTotal;
    }

    public double getDiscountedTotal() {
        return this.discountedTotal;
    }

    public void setDiscount(double number) {
        this.discount = number;
        this.discountedTotal = this.baseTotal - this.discount;
    }
}

해당 코드를 실행하는 테스트 코드를 살펴보면, 버그가 하나있다.

class DiscountTest {

    @Test
    void discount() {
        Discount discount = new Discount(100);
        assertEquals(100, discount.getDiscountedTotal());

        discount.setDiscount(10);
        assertEquals(90, discount.getDiscountedTotal());
    }

}

위에 있는 예제코드들을 비교해 봤을때 알수가 있을 것이다.

setDiscount를 실행하기 전에는 discountedTotal의 변수에 값이 할당되지 않기 때문에 discount.getDiscountedTotal() 해당 메소드로 데이터를 불러올 수 없다.

그래서 아래의 코드를 실행했을때 테스트 코드에 실패했다는 결과를 얻을 수 있다.

assertEquals(100, discount.getDiscountedTotal());

리팩토링001

setDiscount을 실행해야만 discountedTotal의 변수의 값을 가져올수 있다는것은 잘못된 로직이다. 이 버그를 고쳐보도록 하자.

테스트 결과는 assert문을 추가해 테스트코드를 실행시킨 뒤 결과가 기존과 올바른지 확인 해볼수 있다. assert로 get메소드를 만든뒤에 기존의 테스트코드와 동일하게 작동한다면, 변환하고자 하는 코드는 올바르다고 볼 수 있다.

public double getDiscountedTotal() {
    assert this.discountedTotal == this.baseTotal -this.discount;
    return this.discountedTotal;
}

이제 위에서 테스트한 코드를 이용해 getDiscountedTotal을 호출할 때, 아무것도 할당되어있지 않은 discountedTotal 대신에, this.baseTotal -this.discount 의 계산로직을 넣어 할인 결과값을 가져온다.

public class Discount {

    private double discountedTotal;
    private double discount;

    private double baseTotal;

    public Discount(double baseTotal) {
        this.baseTotal = baseTotal;
    }

    public double getDiscountedTotal() {
        return this.baseTotal -this.discount;
    }

    public void setDiscount(double number) {
        this.discount = number;
    }
}

그리고 테스트코드를 실행시키면 버그가 고쳐진것을 알 수 있다.

리팩토링002

두번째 예제를 살펴보자. ProductionPlan 클래스는 간단한 합계를 구하는 예제이다. 해당 코드를 보면 production 변수가 임시적인 계산 변수로 쓰이는것을 볼 수있는데, applyAdjustment 메소드에 들어가는 production 합계 로직은 applyAdjustment 데이터를 받아 적용하는 로직에서 분리하고 제거할 수 있을 것이다.

해당 변수를 제거해주도록 해보자.

public class ProductionPlan {

    private double production;
    private List<Double> adjustments = new ArrayList<>();

    public void applyAdjustment(double adjustment) {
        this.adjustments.add(adjustment);
        this.production += adjustment;
    }

    public double getProduction() {
        return this.production;
    }
}

먼저 assert를 통해 해당 변수를 추출해서 List<Double> adjustments의 합계를 반환하는것으로 메소드를 추출해보자.

public class ProductionPlan {

    private double production;
    private List<Double> adjustments = new ArrayList<>();

    public void applyAdjustment(double adjustment) {
        this.adjustments.add(adjustment);
        this.production += adjustment;
    }

    public double getProduction() {
        assert this.production == calculatedProduction();
        return this.production;
    }

    private double calculatedProduction() {
        return this.adjustments.stream().mapToDouble(Double::valueOf).sum();
    }
}

그리고 테스트코드를 실행해서 기존과 동일하게 작동하는지 확인해보록하자. 기존과 동일하게 동작하는것을 확인할 수 있다.

리팩토링003

assert를 걷어내고 리팩토링한 소스를 적용해준다 또한 인라인 리팩토링을 적용해 분리한 메소드를 제거할 수도 있을것이다. 그리고 이제 사용하지 않는 production 필드를 제거함으로써 변수를 하나 줄였다.

public class ProductionPlan {

    private List<Double> adjustments = new ArrayList<>();

    public void applyAdjustment(double adjustment) {
        this.adjustments.add(adjustment);
    }

    public double getProduction() {
        return this.adjustments.stream().mapToDouble(Double::valueOf).sum();
    }
}

리팩토링 22. 여러함수를 변환 함수로 묶기

  • 관련있는 여러 파생 변수를 만들어내는 함수가 여러곳에서 만들어지고 사용된다면 그러한 파생 변수를 “변환 함수 (transform function)”를 통해 한 곳으로 모아둘 수 있다.
  • 소스 데이터가 변경될 수 있는 경우에는 “여러 함수를 클래스로 묶기 (Combine Functions into Class)”를 사용하는 것이 적절하다.
  • 소스 데이터가 변경되지 않는 경우에는 두 가지 방법을 모두 사용할 수 있지만, 변환 함수를 사용해서 불변 데이터의 필드를 생성해 두고 재사용 할 수도 있다.

설명을 듣자니 너무 어려웠다. 예시를 살펴서 조금더 쉽게 이해해보도록 하자.

각각 Client1, Client2, Client3는 모두 기본 사용료를 계산하는 비슷한 작업을 하고 있다. 다만 조금씩 차이가 난다면, Client2, Client3는 세금으로 공제를 해주는 로직이 추가 되어 있다.

  • Client1
public class Client1 {

    double baseCharge;

    public Client1(Reading reading) {
        this.baseCharge = baseRate(reading.month(), reading.year()) * reading.quantity();
    }

    private double baseRate(Month month, Year year) {
        return 10;
    }

    public double getBaseCharge() {
        return baseCharge;
    }
}
  • Client2
public class Client2 {

    private double base;
    private double taxableCharge;

    public Client2(Reading reading) {
        this.base = baseRate(reading.month(), reading.year()) * reading.quantity();
        this.taxableCharge = Math.max(0, this.base - taxThreshold(reading.year()));
    }

    private double taxThreshold(Year year) {
        return 5;
    }

    private double baseRate(Month month, Year year) {
        return 10;
    }

    public double getBase() {
        return base;
    }

    public double getTaxableCharge() {
        return taxableCharge;
    }
}
  • Client3
public class Client3 {

    private double basicChargeAmount;

    public Client3(Reading reading) {
        this.basicChargeAmount = calculateBaseCharge(reading);
    }

    private double calculateBaseCharge(Reading reading) {
        return baseRate(reading.month(), reading.year()) * reading.quantity();
    }

    private double baseRate(Month month, Year year) {
        return 10;
    }

    public double getBasicChargeAmount() {
        return basicChargeAmount;
    }
}

비슷한 작업을 한다면 중복으로 사용되는 변수 또는 메소드가 있을 것이다. 이것을 공통적인 클래스와 레코드로 빼주자.

먼저 공통적으로 사용되는 taxThresholdbaseRate 메소드를 빼주도록 하자.

public class ReadingClient {

    protected double taxThreshold(Year year) {
        return 5;
    }

    protected double baseRate(Month month, Year year) {
        return 10;
    }
}

그리고 각각 Client1~3 모두 ReadingClient를 상속받는다면 해당 메소드를 빼줄 수 있다.

public class Client1 extends ReadingClient {}
public class Client2 extends ReadingClient {}
public class Client3 extends ReadingClient {}

이제 공통되는 변수들을 제거해 record 필드를 만들어 변환 함수로 묶어볼 것이다. Client1~3에서 공통적으로 Reading을 생성자로 받는것을 볼 수 있을 것이다. 동일하게 Reading을 생성자로 받는 record 필드를 만들어 EnricReading 객체로 뺄것이다. 해당 객체는 immutable(불변)하므로 데이터가 변경될 걱정을 하지 않아도 된다.

protected EnricReading enricReading(Reading reading) {
    return new EnricReading(reading);
}
public record EnricReading(Reading reading) {
}

하지만 해당 Reading만 넘기면 아무것도 하질 않으니, Client1~3에서 자주 사용하는 변수인 baseChargeEnricReading record 에 넣어주어 관리를 쉽게 할수 있도록 한다.

public record EnricReading(Reading reading, double baseCharge) {
}

그렇다면, EnricReading 를 통해 baseCharge 변수를 관리할 수 있게 될것이고, Client3basicChargeAmountEnricReading를 통해 관리할수 있게된다.

public class Client3 extends ReadingClient{

    private double basicChargeAmount;

    public Client3(Reading reading) {
        this.basicChargeAmount = enricReading(reading).baseCharge();
    }

    public double getBasicChargeAmount() {
        return basicChargeAmount;
    }
}

물론 기존에 basicChargeAmount를 계산하던 calculateBaseCharge 메소드는 ReadingClient 클래스에 옮겨주어, record에 baseCharge를 넣는 메소드로 사용하게 한다.

public class ReadingClient {

    protected double taxThreshold(Year year) {
        return 5;
    }

    protected double baseRate(Month month, Year year) {
        return 10;
    }

    protected EnricReading enricReading(Reading reading) {
        return new EnricReading(reading, calculateBaseCharge(reading));
    }

    private double calculateBaseCharge(Reading reading) {
        return baseRate(reading.month(), reading.year()) * reading.quantity();
    }
}

그렇다면 각각 Client1Client2 도 동일하게 basicChargeEnricReading record를 통해서 관리하도록 수정할 수 있다.

public class Client1 extends ReadingClient{

    double baseCharge;

    public Client1(Reading reading) {
        this.baseCharge = enricReading(reading).baseCharge();
    }

    public double getBaseCharge() {
        return baseCharge;
    }
}
public class Client2 extends ReadingClient{

    private double base;
    private double taxableCharge;

    public Client2(Reading reading) {
        this.base = enricReading(reading).baseCharge();
        this.taxableCharge = Math.max(0, this.base - taxThreshold(reading.year()));
    }

    public double getBase() {
        return base;
    }

    public double getTaxableCharge() {
        return taxableCharge;
    }
}

만약 Client2taxableCharge 또한 해당 클래스 뿐만아니라 여러 클래스에서 사용하게 된다면 baseCharge와 동일하게 빼줄 수 있다. 먼저 record에 taxableCharge를 추가해준 뒤 ReadingClient 에서도 taxableCharge를 담아줄 수 있도록 Client2taxableCharge 계산로직을 추출해서 해당 변수를 만들 수 있도록 추가하도록 한다.

public record EnricReading(Reading reading, double baseCharge, double taxableCharge) {
}
public class ReadingClient {

    protected double taxThreshold(Year year) {
        return 5;
    }

    protected double baseRate(Month month, Year year) {
        return 10;
    }

    protected EnricReading enricReading(Reading reading) {
        return new EnricReading(reading, baseCharge(reading), taxableCharge(reading));
    }

    private double taxableCharge(Reading reading) {
        return Math.max(0, baseCharge(reading) - taxThreshold(reading.year()));
    }

    private double baseCharge(Reading reading) {
        return baseRate(reading.month(), reading.year()) * reading.quantity();
    }
}

그렇다면, Client2taxableChargeEnricReading record에서 가져올 수 있도록 변환이 된다.

public class Client2 extends ReadingClient{

    private double base;
    private double taxableCharge;

    public Client2(Reading reading) {
        this.base = enricReading(reading).baseCharge();
        this.taxableCharge = enricReading(reading).taxableCharge();
    }

    public double getBase() {
        return base;
    }

    public double getTaxableCharge() {
        return taxableCharge;
    }
}

해당부분을 다시한번 리팩토링 한다면 enricReading(reading) 부분을 계속적으로 호출해 새로운 인스턴스를 생성하는 것이아니라. EnricReading record를 인스턴스화 해 재사용을 할 수 있도록 수정할 수 있다.

public Client2(Reading reading) {
    EnricReading enricReading = enricReading(reading);
    this.base = enricReading.baseCharge();
    this.taxableCharge = enricReading.taxableCharge();
}

리팩토링 23. 참조를 값으로 바꾸기

  • 레퍼런스 (Reference) 객체 vs 값 (Value) 객체
    • https://martinfowler.com/bliki/ValueObject.html
    • “Objects that are equal due to the value of their properties, in this case their x and y coordinates, are called value objects.”
    • 값 객체는 객체가 가진 필드의 값으로 동일성을 확인한다.
    • 값 객체는 변하지 않는다.
    • 어떤 객체의 변경 내역을 다른 곳으로 전파 시키고 싶다면, 레퍼런스, 아니면 불변 값 객체를 사용한다.

예제코드를 살펴 보도록 하자.

  • TelephoneNumber
public class TelephoneNumber {

    private String areaCode;

    private String number;

    public String areaCode() {
        return areaCode;
    }

    public void areaCode(String areaCode) {
        this.areaCode = areaCode;
    }

    public String number() {
        return number;
    }

    public void number(String number) {
        this.number = number;
    }
}
  • Person
public class Person {

    private TelephoneNumber officeTelephoneNumber;

    public String officeAreaCode() {
        return this.officeTelephoneNumber.areaCode();
    }

    public void officeAreaCode(String areaCode) {
        this.officeTelephoneNumber.areaCode(areaCode);
    }

    public String officeNumber() {
        return this.officeTelephoneNumber.number();
    }

    public void officeNumber(String number) {
        this.officeTelephoneNumber.number(number);
    }

}

TelephoneNumber 클래스의 값은 변경될 수 있는 객체이다. 내부에 setter가 존재하고, Person에서 참조하고, 값을 변경해주고 있기 때문이다.

이제 TelephoneNumber 클래스를 value Object로 변경해주고 싶다면,

먼저 값 객체를 생성할 수 있도록 생성자를 생성하고 내부변수들을 모두 불변으로 선언해준다. 그리고, Setter 메소드를 지움으로써 내부 데이터를 변경할 수 있는 수단을 없앤다. 또한 값 객체는 객체끼리의 비교가 들어가게 되므로, hashcode와, equals를 재정의(Override) 해준다. 같은 값을 가진 객체라면 같은 Value라는 것을 검증 할 수 있도록 해준다.

public class TelephoneNumber {

    final private String areaCode;

    final private String number;

    public TelephoneNumber(String areaCode, String number) {
        this.areaCode = areaCode;
        this.number = number;
    }

    public String areaCode() {
        return areaCode;
    }

    public String number() {
        return number;
    }

    @Override
    public boolean equals(Object o) {
      if (this == o) return true;
      if (o == null || getClass() != o.getClass()) return false;
      TelephoneNumber that = (TelephoneNumber) o;
      return Objects.equals(areaCode, that.areaCode) && Objects.equals(number, that.number);
    }

    @Override
    public int hashCode() {
      return Objects.hash(areaCode, number);
    }
}

테스트는 이후에 진행하도록 하고, 이제 Person의 객체 생성부분을 수정해주자. setter 메소드를 이용해 데이터를 넣어주던 부분을 생성자 생성을 통해 새로운 값 객체를 만들어 준다.

public class Person {

    private TelephoneNumber officeTelephoneNumber;

    public String officeAreaCode() {
        return this.officeTelephoneNumber.areaCode();
    }

    public void officeAreaCode(String areaCode) {
        this.officeTelephoneNumber = new TelephoneNumber(areaCode, this.officeNumber());
    }

    public String officeNumber() {
        return this.officeTelephoneNumber.number();
    }

    public void officeNumber(String number) {
        this.officeTelephoneNumber = new TelephoneNumber(this.officeAreaCode(), number);
    }
}

이제 값 객체의 값이 동일한지 테스트를 진행해보자.

만약 equals를 재정의 하지 않았다면 각각 새로운 인스턴스를 생성한 number1과 number2는 다른 객체일 것이다.

public class TelephoneNumberTest {

    @Test
    void telephone_동일성_비교() {
        TelephoneNumber number1 = new TelephoneNumber("123", "1234");
        TelephoneNumber number2 = new TelephoneNumber("123", "1234");
        assertEquals(number1, number2);
    }
}

결과는 두 객체가 동일한것을 확인 할 수 있다.

리팩토링004

자바 버전 14 이후에서는 TelephoneNumber 객체를 record 객체로 만든다면 내부 코드는 다 지우고 사용 할 수 있다.

public record TelephoneNumberRecord(String areaCode, String number) {
}

마지막으로 두 레코드의 동일성을 테스트 해보도록 하자

@Test
void recordTelephone_동일성_비교() {
    TelephoneNumberRecord number1 = new TelephoneNumberRecord("123", "1234");
    TelephoneNumberRecord number2 = new TelephoneNumberRecord("123", "1234");
    assertEquals(number1, number2);
}

레코드와의 비교도 동일한 값이 나오는것을 확인할 수 있다.

리팩토링004