배경

오늘 함께 일하시는 FE 분께서 API 응답 데이터가 요구사항과 다르다고 확인을 부탁하셨습니다.

제가 작업했던 부분은 아니었으나 해당 부분을 구현하셨던 팀원 분께서 회의 일정이 있어 제가 대응을 하게 되었습니다.


요구사항 자체는 간단했습니다. 주문 상태가 WAIT 인 데이터가 포함되면 안되었기에 해당 부분만 필터링을 추가하면 되는 작업이었습니다.

해당 기능에 대한 코드입니다.

// Before
public List<OrderModel> getOrderModels(List<Market> markets, boolean urgent) {
    Map<Long, Market> marketsById = markets.stream()
                                           .collect(Collectors.toMap(Market::getId, Function.identity()));

    List<Order> orders = getWaitingOrders(marketsById.keySet());

    List<OrderModel> OrderModels = Lists.newArrayList();
    for (Order order : orders) {
        Long ownerId = marketsById.get(order.getMarketId()).getOwnerId();
        OrderModels.add(new OrderModel(order.getMarketId(),
                                       order.getId(),
                                       order.getProduct(),
                                       order.getCreatedDateTime(),
                                       order.getPlace(),
                                       order.getStatus(),
                                       order.getMemberId(),
                                       ownerId));
    }

    if (urgent) { // here
        OrderModels.removeIf(order -> order.getStatus() != OrderStatus.WAIT);
    }

    return OrderModels.stream()
                      .sorted(Comparator.comparing(OrderModel::getId))
                      .limit(READ_SIZE)
                      .collect(Collectors.toList());
}

private List<Order> getWaitingOrders(Collection<Long> marketIds) {
    return orderRepository.findWatingOrdersByMarketIds(marketIds);
}

코드를 보시면 if (urgent) {} 부분에서는 WAIT 상태인 데이터를 필터링하고 있으나, 그 외 상황에서는 WAIT 상태 데이터가 포함되고 있었습니다.


해당 부분을 간단히 수정할 수 있었지만, 전반적으로 가독성을 개선하고, Order → OrderModel 로의 매핑과 정렬 작업이 함께 이루어지면 좋을 것 같다는 생각이 들었습니다.

그래서 OrderStatus 필터링에 대한 조건을 메서드화 하고, 매핑과 정렬 작업을 하나의 Stream으로 작성하였습니다.

// After
public List<OrderModel> getNotAcceptedOrderModels(List<Market> markets, boolean urgent) {
    Map<Long, Market> marketsById = markets.stream()
                                           .collect(Collectors.toMap(Market::getId, Function.identity()));

    List<Order> orders = getWaitingOrders(marketsById.keySet());

    return orders.stream()
                 .map(order -> new OrderModel(order.getMarketId(),
                                              order.getId(),
                                              order.getProduct(),
                                              order.getCreatedDateTime(),
                                              order.getPlace(),
                                              order.getStatus(),
                                              order.getMemberId(),
                                              marketsById.containsKey(order.getMarketId()) ?
                                              marketsById.get(order.getMarketId()).getOwnerId() :
                                              null))
                 .filter(order -> inTargetStatus(order, urgent))
                 .sorted(Comparator.comparing(OrderModel::getId))
                 .limit(READ_SIZE)
                 .collect(Collectors.toList());
}

private List<Order> getWaitingOrders(Collection<Long> marketIds) {
    return orderRepository.findWatingOrdersByMarketIds(marketIds);
}

private boolean inTargetStatus(OrderModel order, boolean urgent) {
    if (urgent) {
        return order.getStatus() == OrderStatus.TIMEOUT;
    }
    return order.getStatus() != OrderStatus.ACCEPTED;
}

리팩토링 후 코드입니다.


Stream이 너무 길어져도 좋지 않을 수 있지만, 반복문이나 Stream이 파편화 되어있으면 관리하기가 어려울 수 있을 것 같습니다.

이를 이용하면 어느 정도는 가독성이 향상될 수 있다고 생각합니다 :)