(인프런 워밍업 스터디 클럽2기 백엔드(클린코드, 테스트 코드)🍀 Day4 미션

**인프런 [Readable Code: 읽기 좋은 코드를 작성하는 사고법 강의 대시보드 - 인프런** (inflearn.com)](https://www.inflearn.com/course/readable-code-%EC%9D%BD%EA%B8%B0%EC%A2%8B%EC%9D%80%EC%BD%94%EB%93%9C-%EC%9E%91%EC%84%B1%EC%82%AC%EA%B3%A0%EB%B2%95/dashboard)강의를 참조하여 작성한 글입니다.

1. 아래 코드와 설명을 보고, [섹션3. 논리, 사고의 흐름]에서 이야기 하는 내용을 중심으로 읽기 좋은 코드로 리팩토링해 봅시다.

public boolean validateOrder(Order order) {
	if (order.getItems().size() == 0) { 
		log.info("주문 항목이 없습니다."); 
		return false; 
	} else {
		 if (order.getTotalPrice() > 0) {
			if (!order.hasCustomerInfo()) {
				   log.info("사용자 정보가 없습니다."); return false; 
			} else{
				return true; 
			} 
		} else if (!(order.getTotalPrice() > 0)) {
			 log.info("올바르지 않은 총 가격입니다."); 
			 return false; 
		} 
	}
	return true;
}

해당 메서드를 보면 생성한 Order가 유효한지 검증하는 코드처럼 보인다. 하지만 중첩 if~else if로 되어있어 읽기 좋지 않은 코드 이다. 해당 메서드에는 Order라는 파라미터가 들어오니 Order 클래스와 order.getItem()이 있는 것을 보니 Item 클래스도 있어야 할 것 같다.

public class Order {  
    private final List<Item> items;  
    private final String customerInfo;  
  
    public Order(List<Item> items, String customerInfo) {  
        this.items = (items.isEmpty()) ? Collections.emptyList() : items;  
        this.customerInfo = (customerInfo == null) ? "" : customerInfo;
    }  
  
    public List<Item> getItems() {  
        return items;  
    }  
  
    public int getTotalPrice() {  
        return items.stream().mapToInt(Item::getPrice).sum();  
    }  
  
    public boolean hasCustomerInfo() {  
        return !this.customerInfo.isEmpty();  
    }  
}

public class OrderService {  
  
    private final Logger log = Logger.getLogger(this.getClass().getName()); 
     
    public boolean validateOrder(Order order) {  
        if (order.getItems().size() == 0) {  
            log.info("주문 항목이 없습니다.");  
            return false;  
        } else {  
            if (order.getTotalPrice() > 0) {  
                if (!order.hasCustomerInfo()) {  
                    log.info("사용자 정보가 없습니다."); return false;  
                } else{  
                    return true;  
                }  
            } else if (!(order.getTotalPrice() > 0)) {  
                log.info("올바르지 않은 총 가격입니다.");  
                return false;  
            }  
        }  
        return true;  
    }  
}

우선 너무 많은 중첩 if 문을 early return 으로 처리 할 수 있는 부분은 처리 해보겠다. (어우 한눈에 안들어온다😒)

public class OrderService {  
  
    private final Logger log = Logger.getLogger(this.getClass().getName());  
  
    public boolean validateOrder(Order order) {  
        if (order.getItems().isEmpty()) {  
            log.info("주문 항목이 없습니다.");  
            return false;  
        }  
        if (order.getTotalPrice() > 0) {  
            if (!order.hasCustomerInfo()) {  
                log.info("사용자 정보가 없습니다.");  
                return false;  
            }  
            return true;  
        }  
        if (!(order.getTotalPrice() > 0)) {  
            log.info("올바르지 않은 총 가격입니다.");  
            return false;  
        }  
        return true;  
    }  
}

흠.. 제거를 했지만 부정 연산자 때문에 의미가 한번에 파악하기 어렵다

public class OrderService {  
  
    private final Logger log = Logger.getLogger(this.getClass().getName());  
  
    public boolean validateOrder(Order order) {  
        if (order.getItems().isEmpty()) {  
            log.info("주문 항목이 없습니다.");  
            return false;  
        }  
  
        if (order.getTotalPrice() > 0) {  
            if (order.hasCustomerInfo())  
                return true;  
  
            log.info("사용자 정보가 없습니다.");  
            return false;  
        }  
  
        if (order.getTotalPrice() > 0) {  
            return true;  
        }  
        
        log.info("올바르지 않은 총 가격입니다.");  
        return false;  
    }  
}

이렇게 하니 early return으로 처리한 if(order.getTotalPrice() > 0)을 제거해도 된다.

public class OrderService {  
  
    private final Logger log = Logger.getLogger(this.getClass().getName());  
  
    public boolean validateOrder(Order order) {  
        if (order.getItems().isEmpty()) {  
            log.info("주문 항목이 없습니다.");  
            return false;  
        }  
  
        if (order.getTotalPrice() > 0) {  
            if (order.hasCustomerInfo())  
                return true;  
  
            log.info("사용자 정보가 없습니다.");  
            return false;  
        }  
  
        log.info("올바르지 않은 총 가격입니다.");  
        return false;  
    }  
}

지금 if문 안에 order.getItem().isEmpty()나 order.getTotalPrice() > 0 굳이 getter를 사용해야 할까? 의문이 든다. Order 클래스 안에서 처리 할 수 있지 않을까?

public class OrderService {  
  
    private final Logger log = Logger.getLogger(this.getClass().getName());  
  
    public boolean validateOrder(Order order) {  
        if (order.hasNoItems()) {  
            log.info("주문 항목이 없습니다.");  
            return false;  
        }  
  
        if (order.isValidTotalPrice()) {  
            if (order.hasCustomerInfo())  
                return true;  
  
            log.info("사용자 정보가 없습니다.");  
            return false;  
        }  
  
        log.info("올바르지 않은 총 가격입니다.");  
        return false;  
    }  
}

public class Order {  
    private final List<Item> items;  
    private final String customerInfo;  
  
    public Order(List<Item> items, String customerInfo) {  
        this.items = (items.isEmpty()) ? Collections.emptyList() : items;  
        this.customerInfo = (customerInfo == null) ? "" : customerInfo;  
    }  
  
    private int getTotalPrice() {  
        return items.stream().mapToInt(Item::getPrice).sum();  
    }  
  
    public boolean hasCustomerInfo() {  
        return !this.customerInfo.isEmpty();  
    }  
  
    public boolean hasNoItems() {  
        return items.isEmpty();  
    }  
  
    public boolean isValidTotalPrice() {  
        return getTotalPrice() > 0;  
    }  
}

중첩 if 문도 마저 제거 하고 부정 연산자도 처리 해주었다.

public class OrderService {  
  
    private final Logger log = Logger.getLogger(this.getClass().getName());  
  
    public boolean validateOrder(Order order) {  
        if (order.hasNoItems()) {  
            log.info("주문 항목이 없습니다.");  
            return false;  
        }  
  
        if (order.isNotValidTotalPrice()) {  
            log.info("올바르지 않은 총 가격입니다.");  
            return false;  
        }  
  
        if (order.hasNotCustomerInfo()){  
            log.info("사용자 정보가 없습니다.");  
            return false;  
        }  
        return true;  
    }  
}

public class Order {  
    private final List<Item> items;  
    private final String customerInfo;  
  
    public Order(List<Item> items, String customerInfo) {  
        this.items = (items.isEmpty()) ? Collections.emptyList() : items;  
        this.customerInfo = (customerInfo == null) ? "" : customerInfo;  
    }  
  
    private int getTotalPrice() {  
        return items.stream().mapToInt(Item::getPrice).sum();  
    }  
  
    public boolean hasNotCustomerInfo() {  
        return this.customerInfo.isEmpty();  
    }  
  
    public boolean hasNoItems() {  
        return items.isEmpty();  
    }  
  
    public boolean isNotValidTotalPrice() {  
        return getTotalPrice() <= 0;  
    } 

이쯤 되니 문득 이런 생각이 들었다. Order를 생성 할 때 유효성을 바로 검증하고 오류가 나면 Exception을 날리면 되지 않을까?

public class Order {  
    private final List<Item> items;  
    private final String customerInfo;  
  
    private Order(List<Item> items, String customerInfo) {  
        this.items = (items.isEmpty()) ? Collections.emptyList() : items;  
        this.customerInfo = (customerInfo == null) ? "" : customerInfo;  
    }  
  
    public static Order create(List<Item> items, String customerInfo) {  
        validateOrder(items, customerInfo);  
        return new Order(items, customerInfo);  
    }  
    private static void validateOrder(List<Item> items, String customerInfo) {  
        hasNoItems(items);  
  
        isNotValidTotalPrice(items);  
  
        hasNotCustomerInfo(customerInfo);  
    }  
  
    private static void hasNotCustomerInfo(String customerInfo) {  
        if (customerInfo.isEmpty()){  
            throw new OrderException("사용자 정보가 없습니다.");  
        }  
    }  
  
    private static void isNotValidTotalPrice(List<Item> items) {  
        int totalPrice = calculateTotalPrice(items);  
        if (totalPrice <= 0) {  
            throw new OrderException("올바르지 않은 총 가격입니다.");  
        }  
    }  
  
    private static void hasNoItems(List<Item> items) {  
        if (items.isEmpty()) {  
            throw new OrderException("주문 항목이 없습니다.");  
        }  
    }  
  
    private static int calculateTotalPrice(List<Item> items) {  
        return items.stream().mapToInt(Item::getPrice).sum();  
    }  
}

이렇게 하고 보니 Order 클래스에서 과도하게 주문 생성 및 초기화 하고 주문 검증하고 총 가격 계산하는 일을 하고 있는데 주문 정보를 유지하고 검증하는 것이 Order 클래스의 책임 이라고 볼 수 있지만 검증 하는 부분을 별도로 분리할 필요가 있어 보인다.

public class Order {  
    private final List<Item> items;  
    private final String customerInfo;  
  
    private Order(List<Item> items, String customerInfo) {  
        this.items = (items.isEmpty()) ? Collections.emptyList() : items;  
        this.customerInfo = (customerInfo == null) ? "" : customerInfo;  
    }  
  
    public static Order create(List<Item> items, String customerInfo) {  
        OrderValidator.validateOrder(items, customerInfo);  
        return new Order(items, customerInfo);  
    }  
}

public class OrderValidator {  
  
    public static void validateOrder(List<Item> items, String customerInfo) {  
        hasNoItems(items);  
        isNotValidTotalPrice(items);  
        hasNotCustomerInfo(customerInfo);  
    }  
  
    private static void hasNotCustomerInfo(String customerInfo) {  
        if (customerInfo == null || customerInfo.isEmpty()){  
            throw new OrderException("사용자 정보가 없습니다.");  
        }  
    }  
  
    private static void isNotValidTotalPrice(List<Item> items) {  
        int totalPrice = calculateTotalPrice(items);  
        if (totalPrice <= 0) {  
            throw new OrderException("올바르지 않은 총 가격입니다.");  
        }  
    }  
  
    private static void hasNoItems(List<Item> items) {  
        if (items.isEmpty()) {  
            throw new OrderException("주문 항목이 없습니다.");  
        }  
    }  
  
    private static int calculateTotalPrice(List<Item> items) {  
        return items.stream().mapToInt(Item::getPrice).sum();  
    }  
}

이제 OrderValidator 테스트와 Order가 잘 만들어지는지 통합 테스트를 작성 해보겠다😘

class OrderValidatorTest {  
    @Test  
    @DisplayName("주문 생성시 아이템이 없으면 예외가 발생한다")  
    void hasNoItemTest() {  
        // given  
        List<Item> items = Collections.EMPTY_LIST;  
        String customerInfo = "고객 정보";  
        // when & then  
        String message = assertThrows(OrderException.class, () -> OrderValidator.validateOrder(items, customerInfo)).getMessage();  
        System.out.println("message = " + message);  
    }  
  
    @Test  
    @DisplayName("주문 생성시 사용자 정보가 없으면 예외가 발생한다")  
    void isNotValidTotalPriceTest() {  
        // given  
        List<Item> items = List.of(new Item("item1",1000));  
        String customerInfo = "";  
        // when & then  
        String message = assertThrows(OrderException.class, () -> OrderValidator.validateOrder(items, customerInfo)).getMessage();  
        System.out.println("message = " + message);  
    }  
  
    @Test  
    @DisplayName("주문 생성시 올바르지 않은 가격 합 일때 예외가 발생한다")  
    void isNotValidTotalPrice() {  
        // given  
        List<Item> items = List.of(new Item("item1",-1000), new Item("item2",1000));  
        String customerInfo = "고객 정보";  
        // when & then  
        String message = assertThrows(OrderException.class, () -> OrderValidator.validateOrder(items, customerInfo)).getMessage();  
        System.out.println("message = " + message);  
    }  
}

성공!😍

class OrderTest {  
    @Test  
    @DisplayName("주문 생성 성공")  
    void OrderTestSuccess() {  
        // given  
        List<Item> items = List.of(new Item("item1",1000), new Item("item2",2000));  
        String customerInfo = "구성재";  
        // when  
        Order order = Order.create(items, customerInfo);  
        // then  
        assertEquals(customerInfo, order.getCustomerInfo());  
        assertEquals(items, order.getItems());  
    }  
  
    @Test  
    @DisplayName("주문 성성 실패")  
    void OrderTestFail() {  
        // given  
        List<Item> items = List.of(new Item("item1",1000), new Item("item2",-1000));  
        String customerInfo = "구성재";  
        // when & then  
        assertThrows(OrderException.class, () -> Order.create(items, customerInfo));  
    }  
}

성공!😍

2. SOLID에 대하여 자기만의 언어로 정리해보자

단일 책임 원칙(SRP)

하나의 클래스는 단 한가지의 변경 이유만(책임) 을 가져야 한다.

가장 핵심적으로 기억해야 하는 건 내가 생각했을때는 해당 객체가 단일 책임에 의해서만 변경이 되는가? 이 부분인 것 같다.

public class Book {
	final static Boolean korBook = true;
	final static Boolean engBook = false;
	Boolean language;
	void translate(){
		 if(this.language == korBook){
		  //그대로 둔다
		 }else{
		  //번역한다. 
		 }
	}
}

Book이라는 class가 있고 translate라는 메서드로 korBook일땐 그대로 두고 engBook일땐 번역하는 것을 볼 수 있다.

여기서 문제는 Book이라는 class의 translate()가 korBook과 engBook의 행위를 모두 구현하려고 하기 때문에 단일 책임(행위) 원칙에 위배가 되고 있다.

개방 폐쇄 원칙(OCP)

자신의 확장에는 열려 있고, 주변의 변화에는 닫혀 있어야 한다.

예를 들어 고객이 물건을 사려고하는데 알바가 물건을 팔수도 있을 것이고 사장님이 팔수도 있을 것이다. 근본적인 판다는 행위는 파는 사람이 아무리 달라져도 바뀌지 않는다. 즉 다양한 Seller가 있다는 것은 파는(인터페이스)입장에서는 확장에 개방돼 있는 것이고, 손님 입장에서는 아무리 Seller가 달라진다고 해도 물건을 정상적으로 살수 있기 때문에 닫혀 있는 것이다.

대표적으로 자바의 경우 JVM이라는 것을 두어 어느 OS에서든 상관없이 실행 할 수 있는 것도 예가 될 수 있겠다.

리스코프 치환 원칙(LSP)

서브 타입은 언제나 자신의 기반 타입으로 교체할 수 있어야 한다.

객체지향의 상속에서

하위 클래스 si a kind of 상위 클래스 - 하위 분류(클래스)는 상위 분류의 한 종류이다
구현 클래스 is able to 인터페이스 - 구현 분류(클래스)는 인터페이스할 수 있어야 한다.

이 조건을 만족한다면 LSP를 이미 만족하고 있다.

예를 들어 Book이라는 상위 클래스 KorBook이라는 하위 클래스가 있다고 하자.

Book book1 = new KorBook();

Book이라는 분류에 KorBook으로써 Book의 역할을 하는데 문제가 없다. 그러나 계층도 조직도의 경우는 이 경우를 만족 시킬 수 없다.

인터페이스 분리 원칙(ISP)

클라이언트는 자신이 사용하지 않는 메서드에 의존 관계를 맺으면 안된다.

단일 책임원칙과 달리 인터페이스로 쪼개는 것을 말한다.

1번 예제 처럼 KorBook과 EngBook()으로 클래스를 쪼개는 것이 아니라 번역이 필요하다면 EngBook의 역할을 하게 인터페이스를 제한하고 번역이 필요하지 않다면 KorBook의 역할을 하게 인터페이스를 제한하는 것을 말한다.

ISP를 이야기 할 때 따라오는 원칙이 있다 인터페이스 최소주의 원칙 즉 인터페이스를 통해 메서드를 외부에 제공할 때는 최소한의 메서드만 제공하라는 것이다.

의존 역전 원칙(DIP)

자신보다 변하기 쉬운 것에 의존하지 마라

자신보다 변하기 쉬운 것에 의존하던 것을 추상화된 인터페이스나 상위 클래스를 두어 변하기 쉬운것의 변화에 영향받지 않게 하는 것

의존성의 역방향 : 고수준, 저수준 모듈이 모두 추상화에 의존하는 것

즉 인터페이스만 알고 있는 상태이다.