Middle+

Review: application code The class contains logic for processing new orders. It retrieves orders and items from the database, then makes an HTTP call to an external Stock microservice, which reserves goods for the order items. If all reservations are successful, the microservice confirms the reservation in Stock and marks the order as processed. The microservice runs in multiple instances, so the Process method can execute in parallel. To prevent processing the same rows, a pessimistic locking mechanism PostgreSQL Select for update ... skip locked is used. The IBus implementation sends messages to RabbitMq. ```csharp public sealed class PendingOrdersProcessor { public async Task PendingOrdersProcessor(ILogger logger, IBus bus, HttpStockService st { var env = Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT"); var cs = _cs[env]; _db = new OrdersRepository(cs); _bus = bus; logger = logger; ``` ```csharp _stockService = stockService; } private readonly ILogger _logger; private readonly IOrdersRepository _db; private readonly IStockService _stockService; private readonly IBus _bus; private static readonly Dictionary<string, string> _cs = new { { "Development", "User ID=root;Password=myPassword;Host=localhost;Port=5432;Database=myDataBase" }, { "Production", "User ID=root;Password=myPassword;Host=prod-db;Port=5432;Database= "Pooling=true;Min Pool Size=0;Max Pool Size=1;" } }; ```

sobes.tech AI

Answer from AI

The provided code has several issues and opportunities for improvement:

Dependency Initialization in Constructor
- In the code, the constructor of PendingOrdersProcessor implicitly initializes the repository and other dependencies using an environment variable. It is better to pass all dependencies through constructor parameters to improve testability and flexibility.
Thread Safety
- Since the Process method may run in parallel in multiple instances, it is important to ensure that access to shared resources (e.g., the database) is properly synchronized.
- Using SELECT FOR UPDATE SKIP LOCKED is a good practice for pessimistic locking and preventing the processing of the same orders multiple times.
Error Handling and Transactions
- It is important to wrap database operations and external service calls in transactions to ensure atomicity.
- If item reservation fails, changes should be rolled back properly, and the order should not be confirmed.
Logging
- Logging errors and important events will help in diagnostics and monitoring.
Configuration
- Storing connection strings in a dictionary _cs with environment key is a decent approach, but it is advisable to move configuration to a separate config file or use a configuration manager.
Example of an improved constructor:

public sealed class PendingOrdersProcessor
{
    private readonly ILogger _logger;
    private readonly IOrdersRepository _db;
    private readonly IStockService _stockService;
    private readonly IBus _bus;

    public PendingOrdersProcessor(ILogger logger, IBus bus, IStockService stockService, IOrdersRepository ordersRepository)
    {
        _logger = logger ?? throw new ArgumentNullException(nameof(logger));
        _bus = bus ?? throw new ArgumentNullException(nameof(bus));
        _stockService = stockService ?? throw new ArgumentNullException(nameof(stockService));
        _db = ordersRepository ?? throw new ArgumentNullException(nameof(ordersRepository));
    }

    // Rest of the order processing logic
}

This makes the class more modular and easier to test.

Also, it is worth checking that the database connection pool is configured correctly (in the example for Production, max pool size=1 — this could become a bottleneck during parallel processing).

Previous Next