냄새 11. 기본형 집착(Primitive Obsession)

애플리케이션이 다루고 있는 도메인에 필요한 기본 타입을 만들지 않고 프로그래밍 언어가 제공하는 기본 타입을 사용하는 경우가 많다.
- 예) 전화번호, 좌표, 돈, 범위, 수량 등
기본형으로는 단위 (인치 vs 미터) 또는 표기법을 표현하기 어렵다.
관련 리팩토링 기술
- “기본형을 객체로 바꾸기 (Replace Primitive with Object)”
- “타입 코드를 서브클래스로 바꾸기 (Replace Type Code with Subclasses)”
- “조건부 로직을 다형성으로 바꾸기 (Replace Conditional with Polymorphism)”
- “클래스 추출하기 (Extract Class)”
- “매개변수 객체 만들기 (Introduce Parameter Object)”

리팩토링 30. 기본형을 객체로 바꾸기 (Replace Primitive with Object)

개발 초기에는 기본형 (숫자 또는 문자열)으로 표현한 데이터가 나중에는 해당 데이터와 관련있는 다양한 기능을 필요로 하는 경우가 발생한다.
- 예) 문자열로 표현하던 전화번호의 지역 코드가 필요하거나 다양한 포맷을 지원하는 경우
- 예) 숫자로 표현하던 온도의 단위 (화씨, 섭씨)를 변환하는 경우.
기본형을 사용한 데이터를 감싸 줄 클래스를 만들면, 필요한 기능을 추가할 수 있다.

Before

@Getter
public class Order {
    private String priority;

    public Order(String priority) {
        this.priority = priority;
    }
}

priority 필드의 경우 단순 문자열로 표현하고 있다

public class OrderProcessor {

    public long numberOfHighPriorityOrders(List<Order> orders) {
        return orders.stream()
                .filter(o -> o.getPriority() == "high" || o.getPriority() == "rush")
                .count();
    }
}

현재는 위의 priority 필드가 기본형 타입이기 때문에 이를 문자열로 비교하며 filter 작업을 수행한다
위와 같은 상황에선 Priority 클래스를 만들어 기존 기본형 타입을 객체 타입으로 바꿔서 사용해준다

After

@AllArgsConstructor
public class Priority {
    private String value;
    private List<String> legalValues = List.of("low", "normal", "hight", "rush");

    public Priority(String value) {
        if (legalValues.contains(value))
            this.value = value;
        else
            throw new IllegalArgumentException();
    }
    private int index() {
        this.legalValues.indexOf(this.value);
    }
}

필요한 기능에 대해 Priority 라는 메서드를 생성하였기 때문에(객체로 바꿨으므로)
이를 사용해서 client 단 에서는 아래와 같이 사용할 수 있다

public class OrderProcessor {
    public long numberOfHighPriorityOrders(List<Order> orders) {
        return orders.stream()
        .filter(o -> o.getPriority().higherTHan(new Priority("normal")))
    }
}

리팩토링 31. 타입 코드를 서브클래스로 바꾸기(Replace Type Code with Subclasses)

비슷하지만 다른 것들을 표현해야 하는 경우, 문자열(String), 열거형 (enum), 숫자 (int) 등으 로 표현하기도 한다.
- 예) 주문 타입, “일반 주문”, “빠른 주문”
- 예) 직원 타입, “엔지니어”, “매니저”, “세일즈”
- 위 예시의 경우 if "일반 주문" then A, else if "빠른 주문" then B .. 와 같은 로직이 들어가는 경우가 자주 발생
타입을 서브클래스로 바꾸는 계기
- 조건문을 다형성으로 표현할 수 있을 때, 서브클래스를 만들고 “조건부 로직을 다형성으로 바꾸기”를 적용한다.
- 특정 타입에만 유효한 필드가 있을 때, 서브클래스를 만들고 “필드 내리기”를 적용한다.
"타입 코드를 서브클래스로 바꾸기" 는 상속을 사용한 리팩토링의 기장 첫 단계라 볼 수 있다

직접 상속이 가능한 경우(direct inheritance)

Before

@ToString
public class Employee {

    private String name;

    private String type;

    public Employee(String name, String type) {
        this.validate(type);
        this.name = name;
        this.type = type;
    }

    private void validate(String type) {
        List<String> legalTypes = List.of("engineer", "manager", "salesman");
        if (!legalTypes.contains(type)) {
            throw new IllegalArgumentException(type);
        }
    }

    public String getType() {
        return type;
    }

}

해당 클래스 내 validate() 메서드에서 legalTypes 를 필드로 가지고 있어 정의되지 않은 타입에 대해선 예외를 던지게 된다

Test

@Test
    void employeeType() {
        assertEquals("engineer", new Employee("keesun", "engineer").getType());
        assertEquals("manager", new Employee("keesun", "manager").getType());
        assertThrows(IllegalArgumentException.class, () -> new Employee("keesun", "wrong type"));
    }

After

@ToString
public abstract class Employee {

    private String name;

    protected Employee(String name) {
        this.name = name;
    }
    public static Employee createEmployee(String name, String type) {
        return switch (type) {
            case "engineer" -> new Engineer(name);
            case "manager" -> new Manager(name);
            default -> throw new IllegalArgumentException(type);
        };
    }
    protected abstract String getType();
}

public class Engineer extends Employee {

    public Engineer(String name) {
        super(name);
    }

    @Override
    public String getType() {
        return "engineer";
    }
}

하위 클래스에서 Type 을 직접 리턴해주고 있어 Employee 클래스 내 validation 로직이 필요가 없어진다

Test

@Test
    void employeeType() {
        assertEquals("engineer", Employee.createEmployee("keesun", "engineer").getType());
        assertEquals("manager", Employee.createEmployee("keesun", "manager").getType());
        assertThrows(IllegalArgumentException.class, () -> Employee.createEmployee("keesun", "wrong type"));
    }

테스트 코드도 일부 변경되지만 생성 방식이 create factory 메서드로 변경이 되었으므로 이것만 변경해주면 동일하게 통과하게 된다

직접 상속이 불가능한 경우(indirect inheritance)

직접 상속이 불가능하므로 별도의 Primitive Type 을 생성하여 이것을 상속받는 식으로 변경할 수 있다

Before

public class Employee {

    private String name;

    private String typeValue;

    public Employee(String name, String type) {
        this.validate(type);
        this.name = name;
        this.typeValue = type;
    }
    private void validate(String type) {
        List<String> legalTypes = List.of("engineer", "manager", "salesman");
        if (!legalTypes.contains(type)) {
            throw new IllegalArgumentException(type);
        }
    }
    public String capitalizedType() {
        return this.typeValue.substring(0, 1).toUpperCase() + this.typeValue.substring(1).toLowerCase();
    }
}

After

public class Employee {

    private String name;
    private EmployeeType type;

    public Employee(String name, String typeValue) {
        this.name = name;
        this.type = this.employeeType(typeValue);
    }
    public EmployeeType employeeType(String typeValue) {
        return switch (typeValue) {
            case "engineer" -> new Engineer();
            case "manager" -> new Manager();
            default -> throw new IllegalArgumentException(typeValue);
        };
    }
    public String capitalizedType() {
        return this.type.capitalizedType();
    }
}

간접적으로 상속을 활용하기 위해 EmployeeType 클래스를 생성하여 두고, Manager, Engineer 등이 이를 상속받아 구현한다.

public class EmployeeType {
    public String capitalizedType() {
        return this.toString().substring(0, 1).toUpperCase() + this.toString().substring(1).toLowerCase();
    }
}

리팩토링 전 예제에서는 Employee 에 모두 구현되어 있던 capitalizedType() 메서드도 분산되어 EmployeeType 내에서 처리할 수 있다

public class Manager extends EmployeeType {
    @Override
    public String toString() {
        return "manager";
    }
}

리팩토링 32. 조건부 로직을 다형성으로 바꾸기(Replace Conditional with Polymorphism)

복잡한 조건식을 상속과 다형성을 사용해 코드를 보다 명확하게 분리할 수 있다.
swich 문을 사용해서 타입에 따라 각기 다른 로직을 사용하는 코드.
기본 동작과 (타입에 따른) 특수한 기능이 섞여있는 경우에 상속 구조를 만들어서 기본 동작을 상위클래스에 두고 특수한 기능을 하위클래스로 옮겨서 각 타입에 따른 “차이점”을 강조할 수 있다.
모든 조건문을 다형성으로 옮겨야 하는가? 단순한 조건문은 그대로 두어도 좋다. 오직 복 잡한 조건문을 다형성을 활용해 좀 더 나은 코드로 만들 수 있는 경우에만 적용한다. (과용 을 조심하자.)

Before (예제 1)

public class Employee {

    private String type;

    private List<String> availableProjects;

    public Employee(String type, List<String> availableProjects) {
        this.type = type;
        this.availableProjects = availableProjects;
    }

    public int vacationHours() {
        return switch (type) {
            case "full-time" -> 120;
            case "part-time" -> 80;
            case "temporal" -> 32;
            default -> 0;
        };
    }

    public boolean canAccessTo(String project) {
        return switch (type) {
            case "full-time" -> true;
            case "part-time", "temporal" -> this.availableProjects.contains(project);
            default -> false;
        };
    }
}

vacationHours() 메서드에서는 switch 문을 사용하여 full-time, part-time, temporal 채용 형태에 따라 다른 로직이 들어가있는 형태이다

Test

class EmployeeTest {

    @Test
    void fulltime() {
        Employee employee = new Employee("full-time", List.of("spring", "jpa"));
        assertEquals(120, employee.vacationHours());
        assertTrue(employee.canAccessTo("new project"));
        assertTrue(employee.canAccessTo("spring"));
    }

    @Test
    void partime() {
        Employee employee = new Employee("part-time", List.of("spring", "jpa"));
        assertEquals(80, employee.vacationHours());
        assertFalse(employee.canAccessTo("new project"));
        assertTrue(employee.canAccessTo("spring"));
    }

    @Test
    void temporal() {
        Employee employee = new Employee("temporal", List.of("jpa"));
        assertEquals(32, employee.vacationHours());
        assertFalse(employee.canAccessTo("new project"));
        assertFalse(employee.canAccessTo("spring"));
        assertTrue(employee.canAccessTo("jpa"));
    }

}

Employee클래스의 로직대로 full-time, part-time, temporal 채용 형태에 따라 근무 시간, 접근 가능한 프로젝트 등이 분산되어 있고
이에 따라 테스트코드도 구현이 되어 있다.

After (예제1)

public abstract class Employee {

    protected List<String> availableProjects;

    public Employee(List<String> availableProjects) {
        this.availableProjects = availableProjects;
    }

    public Employee() {
    }

    public abstract int vacationHours();

    public boolean canAccessTo(String project) {
        return this.availableProjects.contains(project);
    }
}

public class FullTimeEmployee extends Employee {
    @Override
    public int vacationHours() {
        return 120;
    }

    @Override
    public boolean canAccessTo(String project) {
        return true;
    }
}

public class PartTimeEmployee extends Employee {

    public PartTimeEmployee(List<String> availableProjects) {
        super(availableProjects);
    }

    @Override
    public int vacationHours() {
        return 80;
    }

}

상속을 사용한 클래스들로 분리하여 구현을 분리해준다

Test

class EmployeeTest {

    @Test
    void fulltime() {
        Employee employee = new FullTimeEmployee();
        assertEquals(120, employee.vacationHours());
        assertTrue(employee.canAccessTo("new project"));
        assertTrue(employee.canAccessTo("spring"));
    }

    @Test
    void partime() {
        Employee employee = new PartTimeEmployee(List.of("spring", "jpa"));
        assertEquals(80, employee.vacationHours());
        assertFalse(employee.canAccessTo("new project"));
        assertTrue(employee.canAccessTo("spring"));
    }

    @Test
    void temporal() {
        Employee employee = new TemporalEmployee(List.of("jpa"));
        assertEquals(32, employee.vacationHours());
        assertFalse(employee.canAccessTo("new project"));
        assertFalse(employee.canAccessTo("spring"));
        assertTrue(employee.canAccessTo("jpa"));
    }

}

변경된 코드는 별도 클래스로 분리를 해 놓은 것이기 때문에,
이를 직접 사용하는 클라이언트 단에서 생성시 변경이 필요하다.

Before (예제 2)

public class VoyageRating {

    private Voyage voyage;
    private List<VoyageHistory> history;

    public VoyageRating(Voyage voyage, List<VoyageHistory> history) {
        this.voyage = voyage;
        this.history = history;
    }

    public char value() {
        final int vpf = this.voyageProfitFactor();
        final int vr = this.voyageRisk();
        final int chr = this.captainHistoryRisk();
        return (vpf * 3 > (vr + chr * 2)) ? 'A' : 'B';
    }

    private int captainHistoryRisk() {
        int result = 1;
        if (this.history.size() < 5) result += 4;
        result += this.history.stream().filter(v -> v.profit() < 0).count();
        if (this.voyage.zone().equals("china") && this.hasChinaHistory()) result -= 2;
        return Math.max(result, 0);
    }

    private int voyageRisk() {
        int result = 1;
        if (this.voyage.length() > 4) result += 2;
        if (this.voyage.length() > 8) result += this.voyage.length() - 8;
        if (List.of("china", "east-indies").contains(this.voyage.zone())) result += 4;
        return Math.max(result, 0);
    }

    private int voyageProfitFactor() {
        int result = 2;

        if (this.voyage.zone().equals("china")) result += 1;
        if (this.voyage.zone().equals("east-indies")) result +=1 ;
        if (this.voyage.zone().equals("china") && this.hasChinaHistory()) {
            result += 3;
            if (this.history.size() > 10) result += 1;
            if (this.voyage.length() > 12) result += 1;
            if (this.voyage.length() > 18) result -= 1;
        } else {
            if (this.history.size() > 8) result +=1 ;
            if (this.voyage.length() > 14) result -= 1;
        }

        return result;
    }

    private boolean hasChinaHistory() {
        return this.history.stream().anyMatch(v -> v.zone().equals("china"));
    }
}

예제 코드가 길지만, 위 내용중 this.voyage.zone().equals("china") && this.hasChinaHistory() 이 조건을 묻는 부분이 중복 발생된다.
따라서 이걸 추출해내는 리팩토링 예제이다.

After (예제 2)

public class RatingFactory {

    public static VoyageRating createRating(Voyage voyage, List<VoyageHistory> history) {
        if (voyage.zone().equals("china") && hasChinaHistory(history)) {
            return new ChinaExperiencedVoyageRating(voyage, history);
        } else  {
            return new VoyageRating(voyage, history);
        }
    }

    private static boolean hasChinaHistory(List<VoyageHistory> history) {
        return history.stream().anyMatch(v -> v.zone().equals("china"));
    }
}

해당 조건을 바탕으로 별개의 클래스(ChinaExperiencedVoyageRating) 로 로직을 분리해내려면, 이를 생성해주는 쪽에서의 결정이 필요하다
따라서 생성에 대한 책임을 갖는 별도의 Factory 클래스에서 어떤 객체로 생성할 지 결정하게 한다

Test

class VoyageRatingTest {

    @Test
    void westIndies() {
        VoyageRating voyageRating = RatingFactory.createRating(new Voyage("west-inides", 10),
                List.of(new VoyageHistory("east-indies", 5),
                        new VoyageHistory("west-indies", 15),
                        new VoyageHistory("china", -2),
                        new VoyageHistory("west-africa", 7)));
        assertEquals('B', voyageRating.value());
    }

    @Test
    void china() {
        VoyageRating voyageRating = RatingFactory.createRating(new Voyage("china", 10),
                List.of(new VoyageHistory("east-indies", 5),
                        new VoyageHistory("west-indies", 15),
                        new VoyageHistory("china", -2),
                        new VoyageHistory("west-africa", 7)));
        assertEquals('A', voyageRating.value());
    }

}

RatingFactory.createRating() 메소드 내에서 생성 로직이 추가되어있으므로 기존 테스트 코드도 변경해줘야 한다
아래 구현 내용을 옮기는 코드들은 내용이 길어 일부 생략함

public class ChinaExperiencedVoyageRating extends VoyageRating {

    public ChinaExperiencedVoyageRating(Voyage voyage, List<VoyageHistory> history) {
        super(voyage, history);
    }

    @Override
    protected int captainHistoryRisk() {
        int result = super.captainHistoryRisk() - 2;
        return Math.max(result, 0);
    }
}

중복 조건을 제거하기 위해 별도로 클래스를 만들어주고,
captainHistoryRisk() 메서드 상에서 원래 해당 조건이 구현하던 로직을 Override 하여 제공한다

`VoyageRating` 코드 중

    protected int captainHistoryRisk() {
        int result = 1;
        if (this.history.size() < 5) result += 4;
        result += this.history.stream().filter(v -> v.profit() < 0).count();
        return Math.max(result, 0);
    }

해당 조건문이 추출되었으므로 별도로 필요가 없다.

tonykang22 / study

[Refactoring] 냄새 11. 기본형 집착 #25

냄새 11. 기본형 집착(Primitive Obsession)

리팩토링 30. 기본형을 객체로 바꾸기 (Replace Primitive with Object)

Before

After

리팩토링 31. 타입 코드를 서브클래스로 바꾸기(Replace Type Code with Subclasses)

직접 상속이 가능한 경우(direct inheritance)

Before

Test

After

Test

직접 상속이 불가능한 경우(indirect inheritance)

Before

After

리팩토링 32. 조건부 로직을 다형성으로 바꾸기(Replace Conditional with Polymorphism)

Before (예제 1)

Test

After (예제1)

Test

Before (예제 2)

After (예제 2)

Test

`VoyageRating` 코드 중

tonykang22 / study

[Refactoring] 냄새 11. 기본형 집착 #25

냄새 11. 기본형 집착(Primitive Obsession)

리팩토링 30. 기본형을 객체로 바꾸기 (Replace Primitive with Object)

Before

After

리팩토링 31. 타입 코드를 서브클래스로 바꾸기(Replace Type Code with Subclasses)

직접 상속이 가능한 경우(direct inheritance)

Before

Test

After

Test

직접 상속이 불가능한 경우(indirect inheritance)

Before

After

리팩토링 32. 조건부 로직을 다형성으로 바꾸기(Replace Conditional with Polymorphism)

Before (예제 1)

Test

After (예제1)

Test

Before (예제 2)

After (예제 2)

Test

VoyageRating 코드 중

`VoyageRating` 코드 중