이중 for 문 stream API 로 깔끔하게 정리하기

이번에 눈에 들어온 그지같은 코드는 바로 이중 for문입니다. 

 

우선 리팩토링 전 코드부터 보시죠. 

 

package com.hello.capston.service;

import com.hello.capston.entity.*;
import com.hello.capston.repository.ItemDetailRepository;
import com.hello.capston.repository.OrderItemRepository;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.hibernate.TransactionException;
import org.springframework.security.core.parameters.P;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Isolation;
import org.springframework.transaction.annotation.Transactional;

import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

@Service
@RequiredArgsConstructor
@Slf4j
public class OrderItemService {

    private final OrderItemRepository orderItemRepository;
    private final ItemDetailRepository itemDetailRepository;


    private boolean flag = true;
    private Map<String, String> map = new ConcurrentHashMap<>();

    @Transactional
    public Map<String, String> saveUsingTemporaryOrder(List<TemporaryOrder> tOrder, Order order) {
        for (TemporaryOrder temporaryOrder : tOrder) {
            Item findItem = temporaryOrder.getBucket().getItem();
            OrderItem orderItem = new OrderItem(findItem, order, temporaryOrder.getPrice(), temporaryOrder.getCount());

            // 아래의 코드에서 락이 걸렸기 때문에 다음으로 못넘어감
            // 때문에 최종적으로 save() 메서드가 호출되는 곳이 동시성 문제에서 해결될 수 있음
            List<ItemDetail> findItemDetail = itemDetailRepository.findByItemId(findItem.getId());

            try {
                for (ItemDetail itemDetail : findItemDetail) {
                    if (temporaryOrder.getSize().equals(itemDetail.getSize()) && itemDetail.getStock() > 0) {
                        itemDetail.changeStock(temporaryOrder.getCount());
                        itemDetailRepository.save(itemDetail);
                    }
                    else if (temporaryOrder.getSize().equals(itemDetail.getSize()) && itemDetail.getStock() <= 0) {
                        flag = false;
                    }
                }

                if (flag) {
                    orderItemRepository.save(orderItem);
                }
                else {
                    throw new TransactionException("상품의 재고가 없습니다.");
                }
            } catch (TransactionException e) {
                map.put("message", "상품의 재고가 없습니다.");
            }
        }
        return map;
    }
}

 

저는 이중 for문이 필요한 곳에서는 사용되어야한다고 생각합니다만... 문제는 읽기가 굉장히 힘들다는 점이 문제가 됩니다. 

 

물론 stream API도 읽기 힘든건 마찬가지입니다. 하지만 요즘 드는 생각은 들여쓰기는 최대한 줄이는 것이 코드를 보기 편하다는 것을 깨달았습니다. (개인적인 생각입니다.)

 

그래서 이 이중 for문으로 들여쓰기가 마구 되어있는 코드를 최대한 깔끔하게 다듬어보았습니다. 

 

package com.hello.capston.service;

import com.hello.capston.entity.*;
import com.hello.capston.repository.ItemDetailRepository;
import com.hello.capston.repository.OrderItemRepository;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.hibernate.TransactionException;
import org.springframework.security.core.parameters.P;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Isolation;
import org.springframework.transaction.annotation.Transactional;

import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

@Service
@RequiredArgsConstructor
@Slf4j
public class OrderItemService {

    private final OrderItemRepository orderItemRepository;
    private final ItemDetailRepository itemDetailRepository;


    private boolean flag = true;
    private Map<String, String> map = new ConcurrentHashMap<>();

    @Transactional
    public Map<String, String> saveUsingTemporaryOrder(List<TemporaryOrder> tOrder, Order order) {
        for (TemporaryOrder temporaryOrder : tOrder) {
            Item findItem = temporaryOrder.getBucket().getItem();
            OrderItem orderItem = new OrderItem(findItem, order, temporaryOrder.getPrice(), temporaryOrder.getCount());

            // 아래의 코드에서 락이 걸렸기 때문에 다음으로 못넘어감
            // 때문에 최종적으로 save() 메서드가 호출되는 곳이 동시성 문제에서 해결될 수 있음
            List<ItemDetail> findItemDetail = itemDetailRepository.findByItemId(findItem.getId());

            ItemDetail itemDetailWithFilter =
                    findItemDetail.stream()
                            .filter(itemDetail -> temporaryOrder.getSize().equals(itemDetail.getSize()))
                            .findAny()
                            .orElse(new ItemDetail());

            if (itemStockIsOverZero(itemDetailWithFilter)) {
                itemDetailWithFilter.changeStock(temporaryOrder.getCount());
                itemDetailRepository.save(itemDetailWithFilter);
            }
            else {
                flag = false;
            }

            if (flag) {
                orderItemRepository.save(orderItem);
            }
            else {
                map.put("message", "상품의 재고가 없습니다.");
            }
        }
        return map;
    }

    private boolean itemStockIsOverZero(ItemDetail itemDetailWithFilter) {
        return itemDetailWithFilter.getStock() > 0;
    }
}

 

해당 코드를 리팩토링하는 김에 그지같이 되어있던 예외처리도 깔끔하게 바꿨습니다. 

 

이 코드 이외에도 제 프로젝트에 많은 부분을 깔끔하게 리팩토링 하였습니다.