DEV ℧ Developer Diary

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

05 Oct 2022

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

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

  • 데이터를 변경하다보면 예상치 못했던 경과나 해결하기 어려운 버그가 발생하기도 한다. 코드를 변경하다 보면 의도치 않게 다른 코드에 영향이 가는 “사이드 이펙트”를 주의 해야 한다.
  • 함수형 프로그래밍 언어는 데이터를 변경하지 않고 복사본을 전달한다. 하지만 그밖의 프로그래밍 언어는 데이터 변경을 허용하고 있다. 따라서 변경되는 데이터 사용 시 발생할 수 있는 리스크를 관리할 수 있는 방법을 적용하는 것이 좋다.
  • 관련 리팩토링
    • “변수 캡슐화 하기(Encapsulate Variable)”를 적용해 데이터를 변경할 수 있는 메소드를 제한하고 관리할 수 있다.
    • “변수 쪼개기(Split Variable)”을 사용해 여러 데이터를 저장하는 변수를 나눌 수 있다.
    • “코드 정리하기 (Slide Statements)”를 사용해 데이터를 변경하는 코드를 분리하고 피할 수 있다.
    • “함수 추출하기 (Extract Function)”으로 데이터를 변경하는 코드로부터 사이드 이팩트가 없는 코드를 분리할 수 있다.
    • “질의 함수와 변경 함수 분리하기 (Separate Query from Modifier)”를 적용해서 클라이언트가 원하는 경우에만 사이드 이팩트가 있는 함수를 호출 하도록 API를 개선할 수 있다.
    • 가능하다면 “세터 제거하기 (Remove Setting Method)”를 적용한다.
    • 계산해서 알아낼 수 있는 값에는 “파생 변수를 질의 함수로 바꾸기 (Replace Derived Variable with Query)” 를 적용할 수 있다.
    • 변수가 사용되는 범위를 제한하려면 “여러 함수를 클래스로 묶기 (Combine Functions into Class)” 또는 “여러 함수를 변환 함수로 묶기 (Combine Functions into Transform)”을 적용할 수 있다.
    • “참조를 값으로 바꾸기 (Change Reference to Value)”를 적용해서 데이터 일부를 변경하기 보다는 데이터 전체를 교체할 수 있다.

리팩토링 18. 변수 쪼개기

  • 어떤 변수가 여러번 재할당 되어도 적절한 경우
    • 반복문에서 순회하는데 사용하는 변수 또는 인덱스
    • 값을 축적시키는데 사용하는 변수
  • 그밖에 경우에 재할당 되는 변수가 있다면 해당 변수는 여러 용도로 사용되는 것이며 변수를 분리해야 더 이해하기 좋은 코드를 만들 수 있다.
    • 변수 하나 당 하나의 책임(Responsibility)을 지도록 만든다
    • 상수를 활용하자. (자바스크립트의 const, 자바의 final)

예제 코드

  • Haggis
public class Haggis {

    private double primaryForce;
    private double secondaryForce;
    private double mass;
    private int delay;

    public Haggis(double primaryForce, double secondaryForce, double mass, int delay) {
        this.primaryForce = primaryForce;
        this.secondaryForce = secondaryForce;
        this.mass = mass;
        this.delay = delay;
    }

    public double distanceTravelled(int time) {
        double result;
        double acc = primaryForce / mass;
        int primaryTime = Math.min(time, delay);
        result = 0.5 * acc * primaryTime * primaryTime;

        int secondaryTime = time - delay;
        if (secondaryTime > 0) {
            double primaryVelocity = acc * delay;
            acc = (primaryForce + secondaryForce) / mass;
            result += primaryVelocity * secondaryTime + 0.5 * acc * secondaryTime + secondaryTime;
        }

        return result;
    }
}
  • Order
public class Order {

    public double discount(double inputValue, int quantity) {
        if (inputValue > 50) inputValue = inputValue - 2;
        if (quantity > 100) inputValue = inputValue - 1;
        return inputValue;
    }
}
  • Rectangle
public class Rectangle {

    private double perimeter;
    private double area;

    public void updateGeometry(double height, double width) {
        double temp = 2 * (height + width);
        System.out.println("Perimeter: " + temp);
        perimeter = temp;

        temp = height * width;
        System.out.println("Area: " + temp);
        area = temp;
    }

    public double getPerimeter() {
        return perimeter;
    }

    public double getArea() {
        return area;
    }
}

Rectangle는 직사각형의 넓이를 구하는 코드이다.

먼저 Rectangle 코드를 살펴보자. updateGeometry 메소드는 높이와 가로의 길이를 받아 넓이를 재반환 하는 값이다. 해당 메소드 내부의 변수 temp를 보면 넓이를 재반환는 과정에서 어떤 역할을 하는지 쉽게 파악할 수 있을까? 물론 아니라고 본다.

public void updateGeometry(double height, double width) {
    double temp = 2 * (height + width);
    System.out.println("Perimeter: " + temp);
    perimeter = temp;

    temp = height * width;
    System.out.println("Area: " + temp);
    area = temp;
}

그렇다면 첫번째 temp를 perimeter로, 두번째 temp를 area로 변경해서 각 역할을 구분해주도록 하자.

public void updateGeometry(double height, double width) {
    double perimeter = 2 * (height + width);
    System.out.println("Perimeter: " + perimeter);
    this.perimeter = perimeter;

    double area = height * width;
    System.out.println("Area: " + area);
    this.area = area;
}

테스트를 실행시켜 기존과 동일하게 작동하는지도 확인을 해주도록 하자.

리팩토링001

두번째 예시를 살펴보자.

distanceTravelled 해당 함수는 거리를 구하는 함수로 primaryForce(추진력)을 mass(질량)으로 나누어 acc(가속도)를 구하여, 첫번째 와 두번째 추진력으로 이동한 거리를 구해서 더한다? 라고 볼 수 있다. 여기서 메소드를 자세하게 분석하는 것은 우리가 하는일이 아니므로, 간단하게 넘어가도록 하자. 여기서는 주의깊게 볼점은 acc(가속도) 변수가 두번 동일하게 사용되었다는 점이다.

public double distanceTravelled(int time) {
    double result;
    double acc = primaryForce / mass;
    int primaryTime = Math.min(time, delay);
    result = 0.5 * acc * primaryTime * primaryTime;

    int secondaryTime = time - delay;
    if (secondaryTime > 0) {
        double primaryVelocity = acc * delay;
        acc = (primaryForce + secondaryForce) / mass;
        result += primaryVelocity * secondaryTime + 0.5 * acc * secondaryTime + secondaryTime;
    }

    return result;
}

이는 acc를 각각 primaryAcceleration, secondaryAcceleration로 구분해 몇번째 가속도를 구하는것인지 구분해주면 이해하기가 더 편하다.

public double distanceTravelled(int time) {
    double result;
    double primaryAcceleration = primaryForce / mass;
    int primaryTime = Math.min(time, delay);
    result = 0.5 * primaryAcceleration * primaryTime * primaryTime;

    int secondaryTime = time - delay;
    if (secondaryTime > 0) {
        double primaryVelocity = primaryAcceleration * delay;
        double secondaryAcceleration = (primaryForce + secondaryForce) / mass;
        result += primaryVelocity * secondaryTime + 0.5 * secondaryAcceleration * secondaryTime + secondaryTime;
    }

    return result;
}

더나아가 final 키워드를 사용해 불변객체로 만들어주면 해당 변수가 변동될일이 없다는것이 명확해진다.

final double primaryAcceleration = primaryForce / mass;
final double secondaryAcceleration = (primaryForce + secondaryForce) / mass;

동일하게 테스트가 제대로 동작하는지 확인해 주도록 하자.

리팩토링002

마지막으로 하나만 더 살펴보도록 하자.

해당 메소드는 inputValue 변수를 주목하도록 하자. inputValue 변수는 매개변수로 사용되어 계산에 쓰이기도 하고, 최종적인 값으로 반환되기도 한다. 해당 변수를 분리해주도록 하자.

public double discount(double inputValue, int quantity) {
    if (inputValue > 50) inputValue = inputValue - 2;
    if (quantity > 100) inputValue = inputValue - 1;
    return inputValue;
}

계산하는 값과, result값을 분리해 줌으로써 좀더 계산에 사용되는 값과 결과값이 명확한 코드가 될것이다.

public double discount(double inputValue, int quantity) {
    double result = inputValue;

    if (inputValue > 50) result -= 2;
    if (quantity > 100) result -= 1;

    return result;
}

마찬가지로 테스트코드를 돌려 리팩토링 이후에도 로직에 변경이 없음을 확인하도록 하자.

리팩토링003

리팩토링 19. 질의 함수와 변경 함수 분리하기

  • “눈에 띌만한” 사이드 이팩트 없이 값을 조회할 수 있는 메소드는 테스트 하기도 쉽고, 메소드를 이동하기도 편하다.
  • 명령-조회 분리 (command-query separation) 규칙 : 어떤 값을 리턴하는 함수는 사이드 이팩트가 없어야 한다.
  • “눈에 띌만한 (observable) 사이드 이팩트” 여기서 ‘눈에 띌만한’이라는 의미는 아주 분명한 객체의 상태 변화 코드를 말한다.
    • 하지만, 캐시는 중요한 객체 상태 변화는 아니다. 따라서 어떤 메소드 호출로 인해, 캐시 데이터를 변경하더라도 분리할 필요는 없다.

예제코드는 너무 많은관계로.. 중요한 코드만 가져와 설명하도록 한다.

totalOutstanding 메소드는 getTotalOutstandingAndSendBill 메소드의 테스트 코드로, getTotalOutstandingAndSendBill를 실행시켜 금액을 계산한 후 customer에게 메일을 보내는 역할을 한다. 해당 소스를 실행하면 계산과 영수증의 전송은 무조건적으로 동시에 일어나게 된다.

  • Billing
public double getTotalOutstandingAndSendBill() {
    double result = customer.getInvoices().stream()
            .map(Invoice::getAmount)
            .reduce((double) 0, Double::sum);
    sendBill();
    return result;
}

private void sendBill() {
    emailGateway.send(formatBill(customer));
}
  • BillingTest
@Test
void totalOutstanding() {
    Billing billing = new Billing(new Customer("keesun", List.of(new Invoice(20), new Invoice(30))),
            new EmailGateway());
    assertEquals(50d, billing.getTotalOutstandingAndSendBill());
}

그렇다면, 계산의 결과만 보고싶을 땐 어떻게 해야할까? getTotalOutstandingAndSendBill를 분리해서 하나의 역할만 하도록 만들어주자.

먼저 getTotalOutstandingAndSendBill 내부의 sendBill()을 제거하고, private으로 선언된 sendBill을 public으로 바꿔주어 외부에서도 호출 가능하게 바꿔주자. 그리고, result를 바로 return 해줌으로써 결과만 확인 할 수있게 바꿔주자, 물론 메소드 명을 바꿔주는것은 덤이다.

public double getTotalOutstanding() {
    return customer.getInvoices().stream()
            .map(Invoice::getAmount)
            .reduce((double) 0, Double::sum);
}

public void sendBill() {
    emailGateway.send(formatBill(customer));
}

두번째 예제를 살펴보자. Person객체를 반복해 범죄자를 찾아내 알람을 울리는 로직이다. 해당 로직은 조회를 하는 로직과, 알람을 끄는 로직이 같이있다.

public String alertForMiscreant(List<Person> people) {
    for (Person p : people) {
        if (p.getName().equals("Don")) {
            setOffAlarms();
            return "Don";
        }

        if (p.getName().equals("John")) {
            setOffAlarms();
            return "John";
        }
    }

    return "";
}

이럴 경우 테스트를 돌리면, 어디서 알람이 울렸는지 알 수가 없다.

@Test
void alertForMiscreant() {
    Criminal criminal = new Criminal();
    String found = criminal.alertForMiscreant(List.of(new Person("Keesun"), new Person("Don")));
    assertEquals("Don", found);

    found = criminal.alertForMiscreant(List.of(new Person("John"), new Person("Don")));
    assertEquals("John", found);
}

set off alarm
set off alarm

이제 알람을 끄는 로직과, 범죄자를 찾는 로직을 분리하도록 하자. alertForMiscreant 는 알람을 끄는 로직, findMiscreant는 이름을 조회하는 로직으로 분리를 시켰다.

public void alertForMiscreant(List<Person> people) {
    for (Person p : people) {
        if (p.getName().equals("Don")) {
            setOffAlarms();
        }

        if (p.getName().equals("John")) {
            setOffAlarms();
        }
    }
}

public String findMiscreant(List<Person> people) {
    for (Person p : people) {
        if (p.getName().equals("Don")) {
            return "Don";
        }

        if (p.getName().equals("John")) {
            return "John";
        }
    }

    return "";
}

또 심화를 하자면 for문이 반복되는것을 보고 내부 알고리즘을 변경하여 하나로 변경 해줄수 있다. ““를 리턴한다는 것은 범죄자를 찾지 못한것이므로, isBlank가 true 이다, 그러므로 범죄자를 찾은 경우에 알람을 끈다. 라는 방식으로 내부 알고리즘을 변경 할 수 있다.

public void alertForMiscreant(List<Person> people) {
    if (!findMiscreant(people).isBlank())
        setOffAlarms();
}

public String findMiscreant(List<Person> people) {
    for (Person p : people) {
        if (p.getName().equals("Don")) {
            return "Don";
        }

        if (p.getName().equals("John")) {
            return "John";
        }
    }

    return "";
}

리팩토링 20. 세터 제거하기

  • 세터를 제공한다는 것은 해당 필드가 변경될 수 있다는 것을 뜻한다.
  • 객체 생성시 처음 설정된 값이 변경될 필요가 없다면 해당 값을 설정할 수 있는 생성자를 만들고 세터를 제거해서 변경될 수 있는 가능성을 제거해야 한다.

매우 간단한 리팩토링이다. 먼저 예제를 살펴보도록 하자. Setter의 경우 최대한 안만들고 필요한 경우에만 만드는 것이 좋은데, 그이유는 무분별한 객체의 변경을 막음으로써 데이터의 일관성을 높히는 것이다.

public class Person {

    private String name;

    private int id;

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }

    public int getId() {
        return id;
    }

    public void setId(int id) {
        this.id = id;
    }
}

그렇다고 무조건적으로 setter를 삭제하는 것이 아닌, 아래 테스트 코드를 보면 name의 필드에는 수정이 필요하다. 이때 수정할 필요가 없는 id는 setter를 제거해주고 name에만 데이터를 변경할 수 있도록 setter를 생성해준다.

최대한 생성자를 만들어 데이터를 담은 객체를 생성하고 수정을 하지 않는 것이 데이터를 일관시키는데 좋다.

  • PersonTest
class PersonTest {
    @Test
    void person() {
        Person person = new Person("keesun", 10);
        assertEquals(10, person.getId());
        assertEquals("keesun", person.getName());
        person.setName("whiteship");
        assertEquals("whiteship", person.getName());
    }
}
  • Person
public class Person {

    private String name;

    private int id;

    public Person(String name, int id) {
        this.name = name;
        this.id = id;
    }

    public String getName() {
        return name;
    }

    public int getId() {
        return id;
    }

    public void setName(String name) {
        this.name = name;
    }
}

Related posts