Сделать рефакторинг сервиса создания заказа

16. Сделать рефакторинг сервиса создания заказа

Условие задачи:
📌 Дан сервис OrderService. Нужно сделать рефакторинг: исправить ошибки в коде, улучшить читаемость, валидацию и корректность работы при создании заказа.

Код:

@Service
public class OrderService {

    @Autowired
    private OrderRepository orderRepository;

    @Autowired
    private ProductRepository productRepository;

    public Order createOrder(String userId, List<Product> products, String promoCode) {
        if (products.size() > 0) {
            double total = 0;
            for (int i = 0; i <= products.size(); i++) {
                Integer stock = productRepository.findStockById(products.get(i).getId());

                if (stock == null || stock <= 0) {
                    throw new IllegalStateException("Product out of stock: " + products.getId());
                }
            }
            for (int i = 0; i <= products.size(); i++) {
                total += products.get(i).getPrice();
                productRepository.updateStockById(products.get(i).getId(), 1);
            }
            if (promoCode == "WELCOME10") {
                total = total * 0.9;
            }
            Order order = new Order();
            order.setUserId(userId);
            order.setTotal(total);
            order.setStatus("new");
            System.out.println("Created order for user: " + userId);
            return orderRepository.save(order);
        }
        return null;
    }
}

Спойлеры к решению

Подсказки

💡 @Autowired на полях лучше заменить на constructor injection.
💡 В циклах ошибка: i <= products.size() приведёт к IndexOutOfBoundsException.
💡 products.getId() не скомпилируется, нужен product.getId().
💡 promoCode == "WELCOME10" сравнивает ссылки, а не значения строк.
💡 Возврат null из createOrder() — плохой контракт метода.
💡 Стоит добавить валидацию userId и products.
💡 Обновление остатков и сохранение заказа лучше выполнять в транзакции.
💡 System.out.println(...) лучше заменить на логгер.
💡 Для денег в реальном коде лучше использовать BigDecimal, а не double.

Решение

@Service
@RequiredArgsConstructor
public class OrderService {

    private static final String WELCOME10 = "WELCOME10";

    private final OrderRepository orderRepository;
    private final ProductRepository productRepository;

    @Transactional
    public Order createOrder(String userId, List<Product> products, String promoCode) {
        validate(userId, products);

        double total = 0;

        for (Product product : products) {
            Integer stock = productRepository.findStockById(product.getId());

            if (stock == null || stock <= 0) {
                throw new IllegalStateException("Product out of stock: " + product.getId());
            }

            total += product.getPrice();
            productRepository.updateStockById(product.getId(), stock - 1);
        }

        if (WELCOME10.equals(promoCode)) {
            total = total * 0.9;
        }

        Order order = new Order();
        order.setUserId(userId);
        order.setTotal(total);
        order.setStatus("NEW");

        return orderRepository.save(order);
    }

    private void validate(String userId, List<Product> products) {
        if (userId == null || userId.isBlank()) {
            throw new IllegalArgumentException("userId must not be blank");
        }

        if (products == null || products.isEmpty()) {
            throw new IllegalArgumentException("products must not be empty");
        }
    }
}

В исходном коде были баги с циклами, сравнением строк через ==, return null, ручным field injection и отсутствием транзакции. После рефакторинга сервис стал короче, безопаснее и понятнее. Если говорить глубже, то для денег лучше BigDecimal, а для уменьшения остатков желательно ещё сделать атомарное обновление на уровне БД.