음식_주문 리팩토링

기존 코드

@Transactional
    public OrderDataDto foodOrder(FoodOrderRequestDto requestDto) {

        // 식당 정보 조회
        Restaurant restaurant = restaurantRepository.findById(requestDto.getRestaurantId()).orElseThrow(
                () -> new NullPointerException("존재하지 않는 음식점입니다")
        );

        // 주문 저장
        Order savedOrder = orderRepository.save(new Order(restaurant));

        // 주문_음식 저장
        List<FoodOrderInfoDto> foodOrderInfoList = requestDto.getFoods();
        List<OrderFood> orderFoodList = new ArrayList<>();
        Integer totalPrice = 0;
        List<FoodOrderSearchResultDto> foodResultList = new ArrayList<>();

        for (FoodOrderInfoDto foodOrderInfo : foodOrderInfoList) {
            Food food = foodRepository.findById(foodOrderInfo.getId()).orElseThrow(
                    () -> new NullPointerException("존재하지 않는 음식입니다")
            );

            orderFoodList.add(new OrderFood(savedOrder, food, foodOrderInfo));
            foodResultList.add(new FoodOrderSearchResultDto(food, foodOrderInfo));
            totalPrice += foodResultList.get(foodResultList.size() - 1).getPrice();
        }

        orderFoodRepository.saveAll(orderFoodList);
		
        // 응답 데이터 리턴
        return new OrderDataDto(restaurant, foodResultList, totalPrice);
    }

의문

잘 작성한 코드일까?

내가 지금까지 접했던 리팩토링의 방법들은 아래와 같다.

1. 함수 선언 변경하기
2. 변수 이름 바꾸기
3. 필드 이름 바꾸기
4. 함수 추출 하기
5. 코드 정리하기
6. 메소드 올리기
7. 임시 변수를 질의 함수로 바꾸기
8. 매개변수 객체 만들기
9. 객체 통째로 넘기기
10. 함수를 명령으로 바꾸기
11. 조건문 분해하기
12. 반복문 쪼개기
13. 조건문을 다형성으로 바꾸기
14. 매개변수를 질의 함수로 바꾸기
15. 플래그 인수 제거하기
16. 여러 함수를 클래스로 묶기
17. 변수 캡슐화하기
18. 변수 쪼개기
19. 질의 함수와 변경 함수 분리하기
20. 세터 제거하기
21. 파생 변수를 질의 함수로 바꾸기

리팩토링의 방법은 이외에도 더 있다.. 그래도 우선 위 21가지 방법을 통해서
위에 보이는 코드를 보기 좋게 변경할 수 있는지 고민해 봐야겠다.

---

첫번째 변경 코드

@Transactional // 메서드명 변경 foodOrder -> saveOrderFood
    public OrderDataDto saveOrderFood(OrderFoodRequestDto requestDto) throws Exception {

        // 음식점 이름, 배달비 정보가 필요
        // 음식점 조회
        Restaurant restaurant = getRestaurant(requestDto.getRestaurantId());

        // 주문 저장,
        // '주문_음식' 테이블은 '주문' 테이블과 다대일 관계이기 때문에 주문에 관한 정보를 필요로 함
        Order savedOrder = saveOrder(restaurant);

        //// 주문_음식 저장 ////
        // '주문_음식' 정보를 저장할 리스트
        List<OrderFood> orderFoodList = new ArrayList<>();

        // 사용자 요청 정보를 dto로부터 가져옴
        List<OrderFoodInfoDto> orderFoodInfoList = requestDto.getOrderFoodInfoList();

        // 응답에 필요한 '주문_음식' 정보 리스트
        List<OrderFoodSearchInfo> orderFoodSearchList = new ArrayList<>();

        for (OrderFoodInfoDto orderFoodInfo : orderFoodInfoList) {
            // 특정 음식 정보 조회
            Food food = getFood(orderFoodInfo);

            // 응답 정보 축적
            orderFoodSearchList.add(new OrderFoodSearchInfo(food, orderFoodInfo));

            // '주문_음식' 저장 정보 리스트 축적
            orderFoodList.add(new OrderFood(savedOrder, food, orderFoodInfo));
        }

        // '주문_음식' 정보 저장
        orderFoodRepository.saveAll(orderFoodList);

        return new OrderDataDto(restaurant, orderFoodSearchList);
    }

• Food로 시작하는 클래스, 메서드, 변수명을 모두 Order로 시작하게 바꾸어 주었다. 주문이라는 행위를 통해서 주문한 음식을 처리하는 일련의 로직이 진행되는 것이기 때문에 Food~~ 스러운 클래스, 메서드 그리고 변수명 보다 Order로 시작하는 명칭이 역할을 명확하게 전달 할 수 있다고 판단했기 때문이다. 완전히는 아니지만 리팩토링의 이해하기 힘든 이름에서 착안했다.

• '식당 정보 조회'와 '주문 저장' 코드를 질의 함수로 변경했다.
  이유는 코드가 간결해지는 효과와 주석을 넣지 않더라도 함수를 통해서 무엇을 하는 것인지 전달할 수 있기 때문이었다. 리팩토링의 7, 14 부분에서 배우고 적용해보았다.

• 하나의 주문의 총 합계인 'totalPrice'라는 변수를 없앴다.
  마지막 줄 'OrderDataDto'의 생성자의 인자를 보면 'orderFoodSearchList'와 기존 코드에는 'totalPrice'가 있는 것을 볼 수 있지만 주문의 총 합계는 'orderFoodSearchList'를 통해서도 구할 수 있다. 다른 인자라곤 하여도 기능적으로는 중복 매개 변수가 될 수 있다고 판단되었다. 또한 'totalPrice'를 'saveOrderFood' 메서드에서 없앰으로써 'saveOrderFood'의 책임을 줄일 수도 있었다.

---

두번째 변경 코드

    @Transactional
    public OrderFoodResponseData saveOrderFood(OrderFoodRequestDto requestDto) throws Exception {

        // 음식점 조회
        Restaurant restaurant = getRestaurant(requestDto.getRestaurantId());

        // 주문 저장
        Order savedOrder = saveOrder(restaurant);

        // 주문_음식 저장
        List<OrderFood> savedOrderFoodList = saveOrderFoodByOrder(requestDto, savedOrder);

        // 응답 데이터 리턴
        return new OrderFoodResponseData(restaurant, savedOrderFoodList);
    }

• 'saveOrderFood'라는 메서드는 위 코드의 주석에서 보는 것처럼 '음식점 조회', '주문 저장', '주문_음식 저장'의 순으로만 진행되는 것이 딱 'saveOrderFood'의 역할이라 생각되었다. Entity 모델링 상으로도 이 순서가 맞다. 따라서 '응답 데이터'가 'saveOrderFood'에서 얻게 되는 결과로 만들어질 수 있게 해야한다는 생각을 했고 실제로도 그게 가능하다. 결과적으로는 메서드가 간결해지고 이름에 걸맞는 역할을 수행하는 것으로 보인다는 생각이 들었다.

• '주문_음식 저장'하는 부분을 질의 함수로 변경했다.

---

솔직히 잘한 것인지는 확신할 수 없다. 더 좋은 방법을 찾아서 계속적으로 적용해야 할 것 같다.