C# – Best Practices

This page covers the practices that distinguish production-grade C# from code that merely compiles. These standards apply to all .cs files at Cygnus Dynamics across all layers — controllers, services, repositories, and domain models.

XML Documentation Comments

All public and protected members must have XML documentation comments. These are processed by Visual Studio IntelliSense, Rider, and documentation generators.

Class Documentation

/// <summary>
/// Manages the full lifecycle of customer orders from creation through delivery.
/// </summary>
/// <remarks>
/// This service is the single authoritative entry point for all order operations.
/// It coordinates with <see cref="IPaymentService"/> and <see cref="IInventoryService"/>
/// to ensure transactional consistency across the checkout flow.
/// </remarks>
public class OrderService
{

Method Documentation

/// <summary>
/// Retrieves an order by its unique identifier.
/// </summary>
/// <param name="orderId">The unique identifier of the order. Must be greater than zero.</param>
/// <param name="cancellationToken">Token to cancel the asynchronous operation.</param>
/// <returns>The order matching the given identifier.</returns>
/// <exception cref="OrderNotFoundException">
/// Thrown when no order with the specified <paramref name="orderId"/> exists.
/// </exception>
/// <exception cref="ArgumentOutOfRangeException">
/// Thrown when <paramref name="orderId"/> is less than or equal to zero.
/// </exception>
public async Task<Order> GetOrderAsync(int orderId, CancellationToken cancellationToken = default)
{
    ArgumentOutOfRangeException.ThrowIfNegativeOrZero(orderId);

    return await _repository.FindByIdAsync(orderId, cancellationToken)
        ?? throw new OrderNotFoundException(orderId);
}

Property Documentation

/// <summary>Gets the total cost of all items in the order, before tax.</summary>
public decimal Subtotal => Items.Sum(i => i.UnitPrice * i.Quantity);

/// <summary>Gets or sets the ISO 4217 currency code for this order.</summary>
/// <example>"USD", "EUR", "GBP"</example>
public required string Currency { get; init; }

Update documentation when you change behaviour

An XML comment that contradicts the method's actual behaviour misleads every caller. When you change a method, update the doc comment in the same commit.

Exception Handling

Use Specific Exception Types

// ✅ Specific catch — handles the expected failure precisely
try
{
    var order = await _repository.FindByIdAsync(orderId, ct);
    if (order is null)
        throw new OrderNotFoundException(orderId);
    return order;
}
catch (SqlException ex) when (ex.Number == 1205)  // deadlock
{
    _logger.LogWarning("Deadlock on order {OrderId} — retrying", orderId);
    throw new TransientDatabaseException("Deadlock detected", ex);
}

// ❌ Too broad — hides bugs
try { ... }
catch (Exception ex)   // catches NullReferenceException, OutOfMemoryException, everything
{
    _logger.LogError(ex, "Something failed");
    return null;
}

Exception Filters with `when`

// ✅ Only catches specific HTTP status ranges
try
{
    await _httpClient.PostAsync(endpoint, content, ct);
}
catch (HttpRequestException ex) when ((int?)ex.StatusCode is >= 500 and < 600)
{
    throw new ExternalServiceException("Upstream server error", ex);
}
catch (HttpRequestException ex) when ((int?)ex.StatusCode == 429)
{
    throw new RateLimitExceededException("Too many requests to external service", ex);
}

Custom Exception Classes

Define a typed exception for every distinct domain error. Include the failing identifier as a property:

// ✅ Typed exception with identifier property
public sealed class OrderNotFoundException : Exception
{
    public int OrderId { get; }

    public OrderNotFoundException(int orderId)
        : base($"Order not found: orderId={orderId}")
    {
        OrderId = orderId;
    }

    public OrderNotFoundException(int orderId, Exception inner)
        : base($"Order not found: orderId={orderId}", inner)
    {
        OrderId = orderId;
    }
}

// ✅ Exception hierarchy — allows broad or specific catching
public abstract class OrderException : Exception
{
    protected OrderException(string message) : base(message) { }
    protected OrderException(string message, Exception inner) : base(message, inner) { }
}

public sealed class OrderNotFoundException         : OrderException { ... }
public sealed class OrderAlreadyCancelledException : OrderException { ... }
public sealed class InvalidOrderStateException     : OrderException { ... }

Global Exception Handling in ASP.NET Core

// Program.cs — register global handler using IExceptionHandler (.NET 8+)
public sealed class GlobalExceptionHandler : IExceptionHandler
{
    private readonly ILogger<GlobalExceptionHandler> _logger;

    public GlobalExceptionHandler(ILogger<GlobalExceptionHandler> logger)
        => _logger = logger;

    public async ValueTask<bool> TryHandleAsync(
        HttpContext httpContext,
        Exception exception,
        CancellationToken ct)
    {
        var (statusCode, code) = exception switch
        {
            OrderNotFoundException => (StatusCodes.Status404NotFound, "ORDER_NOT_FOUND"),
            ValidationException    => (StatusCodes.Status400BadRequest, "VALIDATION_ERROR"),
            UnauthorisedAccessException => (StatusCodes.Status403Forbidden, "FORBIDDEN"),
            _ => (StatusCodes.Status500InternalServerError, "INTERNAL_ERROR"),
        };

        var logLevel = statusCode >= 500 ? LogLevel.Error : LogLevel.Warning;
        _logger.Log(logLevel, exception, "Request failed: {Code}", code);

        httpContext.Response.StatusCode = statusCode;
        await httpContext.Response.WriteAsJsonAsync(new
        {
            Code    = code,
            Message = statusCode < 500 ? exception.Message : "An unexpected error occurred.",
            TraceId = Activity.Current?.Id ?? httpContext.TraceIdentifier,
        }, ct);

        return true;
    }
}

// Registration in Program.cs
builder.Services.AddExceptionHandler<GlobalExceptionHandler>();
builder.Services.AddProblemDetails();
app.UseExceptionHandler();

Dependency Injection

Constructor Injection

// ✅ Constructor injection — all dependencies explicit, all fields readonly
public class OrderService
{
    private readonly IOrderRepository _orderRepository;
    private readonly IPaymentService _paymentService;
    private readonly ILogger<OrderService> _logger;

    public OrderService(
        IOrderRepository orderRepository,
        IPaymentService paymentService,
        ILogger<OrderService> logger)
    {
        ArgumentNullException.ThrowIfNull(orderRepository);
        ArgumentNullException.ThrowIfNull(paymentService);
        ArgumentNullException.ThrowIfNull(logger);

        _orderRepository = orderRepository;
        _paymentService = paymentService;
        _logger = logger;
    }
}

// ✅ Primary constructor (C# 12) — more concise
public class OrderService(
    IOrderRepository orderRepository,
    IPaymentService paymentService,
    ILogger<OrderService> logger)
{
    private readonly IOrderRepository _orderRepository = orderRepository
        ?? throw new ArgumentNullException(nameof(orderRepository));
    private readonly IPaymentService _paymentService = paymentService
        ?? throw new ArgumentNullException(nameof(paymentService));
    private readonly ILogger<OrderService> _logger = logger
        ?? throw new ArgumentNullException(nameof(logger));
}

Service Lifetimes

Lifetime	Use When	Example
`Scoped`	One instance per HTTP request	`OrderService`, `DbContext`
`Transient`	New instance every time requested	Lightweight stateless helpers
`Singleton`	Shared across the application lifetime	`IMemoryCache`, connection pools

// Program.cs
builder.Services.AddScoped<IOrderRepository, OrderRepository>();
builder.Services.AddScoped<IOrderService, OrderService>();
builder.Services.AddScoped<IPaymentService, StripePaymentService>();
builder.Services.AddSingleton<IEmailTemplateCache, EmailTemplateCache>();

Configuration with `IOptions<T>`

Use the Options pattern for configuration binding. Never inject IConfiguration directly into services — it bypasses validation and is difficult to test.

// ✅ Strongly-typed options class
public sealed class StripeOptions
{
    public const string Section = "Stripe";

    public required string SecretKey { get; init; }
    public required string WebhookSecret { get; init; }
    public int TimeoutSeconds { get; init; } = 30;
}

// ✅ Register and validate at startup
builder.Services
    .AddOptions<StripeOptions>()
    .BindConfiguration(StripeOptions.Section)
    .ValidateDataAnnotations()
    .ValidateOnStart();   // fails fast at startup if config is missing

// ✅ Inject via IOptions<T> — immutable, snapshot at registration time
public class StripePaymentService
{
    private readonly StripeOptions _options;

    public StripePaymentService(IOptions<StripeOptions> options)
        => _options = options.Value;
}

// ✅ Inject via IOptionsSnapshot<T> — re-read per request (for hot-reload scenarios)
public class FeatureFlagService
{
    private readonly FeatureFlagOptions _options;

    public FeatureFlagService(IOptionsSnapshot<FeatureFlagOptions> options)
        => _options = options.Value;
}

// ❌ Inject IConfiguration directly — no type safety, no validation
public class StripePaymentService
{
    public StripePaymentService(IConfiguration config)
    {
        var key = config["Stripe:SecretKey"];  // nullable, no validation, not testable
    }
}

Sealed Classes

Mark leaf classes sealed by default. Unsealed classes can be subclassed in ways you did not intend, creating unexpected coupling. Only remove sealed when inheritance is an intentional design decision.

// ✅ sealed by default — all concrete implementations
public sealed class OrderService : IOrderService { }
public sealed class StripePaymentGateway : IPaymentGateway { }
public sealed class SqlOrderRepository : IOrderRepository { }

// ✅ sealed on custom exceptions
public sealed class OrderNotFoundException : Exception { ... }

// ✅ Abstract/base classes are not sealed
public abstract class RepositoryBase<T> { }
public abstract class DomainEvent { }

// ❌ Unsealed without intent — anyone can subclass OrderService
public class OrderService : IOrderService { }   // did you mean for this to be extensible?

sealed also enables JIT optimisations

The .NET JIT can devirtualise calls on sealed types, eliminating the overhead of virtual dispatch. Sealing leaf classes is both a correctness and a performance decision.

IDisposable and Resource Management

Implement IDisposable when your class owns unmanaged resources or IDisposable members. Use the standard dispose pattern:

// ✅ Full disposable pattern for classes that own both managed and unmanaged resources
public sealed class OrderExportService : IDisposable
{
    private readonly HttpClient _httpClient;
    private FileStream? _exportStream;
    private bool _disposed;

    public OrderExportService(IHttpClientFactory factory)
        => _httpClient = factory.CreateClient("exports");

    public async Task ExportAsync(string path, CancellationToken ct)
    {
        ObjectDisposedException.ThrowIf(_disposed, this);
        _exportStream = File.Create(path);
        await WriteOrdersAsync(_exportStream, ct);
    }

    public void Dispose()
    {
        if (_disposed) return;
        _exportStream?.Dispose();
        _httpClient.Dispose();
        _disposed = true;
    }
}

// ✅ Always prefer using declaration over manual Dispose
using var service = new OrderExportService(factory);
await service.ExportAsync("/exports/orders.csv", ct);
// Disposed automatically when scope exits

// ✅ IAsyncDisposable for async cleanup (database connections, streams)
public sealed class ReportGenerator : IAsyncDisposable
{
    private readonly NpgsqlConnection _connection;

    public ReportGenerator(string connectionString)
        => _connection = new NpgsqlConnection(connectionString);

    public async ValueTask DisposeAsync()
        => await _connection.DisposeAsync();
}

await using var generator = new ReportGenerator(connectionString);

Entity Framework Core Patterns

DbContext Registration and Scoping

// ✅ Register DbContext as Scoped (default and correct for web apps)
builder.Services.AddDbContext<AppDbContext>(options =>
    options.UseNpgsql(
        builder.Configuration.GetConnectionString("DefaultConnection"),
        npgsql => npgsql
            .MigrationsAssembly("CygnusDynamics.Infrastructure")
            .EnableRetryOnFailure(3)));

// ✅ Use DbContextFactory for background services (Singleton scope)
builder.Services.AddDbContextFactory<AppDbContext>();

// In a background service
public class OrderArchiveService(IDbContextFactory<AppDbContext> factory) : BackgroundService
{
    protected override async Task ExecuteAsync(CancellationToken ct)
    {
        await using var context = await factory.CreateDbContextAsync(ct);
        await ArchiveOldOrdersAsync(context, ct);
    }
}

Querying Patterns

// ✅ AsNoTracking for read-only queries — significant performance gain
public async Task<IReadOnlyList<OrderDto>> GetOrderSummariesAsync(
    int userId,
    CancellationToken ct)
{
    return await _context.Orders
        .AsNoTracking()                                    // no change tracking needed
        .Where(o => o.UserId == userId && o.DeletedAt == null)
        .OrderByDescending(o => o.CreatedAt)
        .Select(o => new OrderDto(o.Id, o.Total, o.Status, o.CreatedAt))
        .ToListAsync(ct);
}

// ✅ Use projection (Select) — never load entities you only need to read
// ❌ Loading full entity for a read-only operation wastes memory
var orders = await _context.Orders.Where(...).ToListAsync(ct);  // loads all columns
var dtos = orders.Select(o => new OrderDto(...));

// ✅ Splitting large includes to avoid query multiplications
var order = await _context.Orders
    .Where(o => o.Id == orderId)
    .AsSplitQuery()       // separate SQL queries for each Include, avoids cartesian explosion
    .Include(o => o.Items)
    .Include(o => o.Payments)
    .Include(o => o.Shipments)
    .FirstOrDefaultAsync(ct);

// ✅ Bulk operations without loading entities (EF Core 7+)
await _context.Orders
    .Where(o => o.CreatedAt < cutoff && o.Status == OrderStatus.Cancelled)
    .ExecuteDeleteAsync(ct);

await _context.Orders
    .Where(o => o.Status == OrderStatus.Pending && o.CreatedAt < timeout)
    .ExecuteUpdateAsync(s => s.SetProperty(o => o.Status, OrderStatus.Expired), ct);

High-Performance Patterns — `Span<T>` and `Memory<T>`

Use Span<T> and Memory<T> for zero-allocation processing of arrays, strings, and buffers in hot code paths:

// ✅ Span<T> — zero-allocation slice of an array or string
public static bool TryParseOrderReference(
    ReadOnlySpan<char> input,
    out int orderId)
{
    // Input: "ORD-000042"
    // Parse without allocating a substring
    var prefixLen = "ORD-".Length;
    if (input.Length <= prefixLen || !input.StartsWith("ORD-"))
    {
        orderId = 0;
        return false;
    }

    return int.TryParse(input[prefixLen..], out orderId);
}

// ✅ MemoryPool<T> for reusable buffers in high-throughput scenarios
public async Task<int> ReadChunksAsync(Stream stream, CancellationToken ct)
{
    using var memoryOwner = MemoryPool<byte>.Shared.Rent(4096);
    var buffer = memoryOwner.Memory;
    int totalRead = 0;

    while (true)
    {
        int bytesRead = await stream.ReadAsync(buffer, ct);
        if (bytesRead == 0) break;
        totalRead += bytesRead;
        ProcessChunk(buffer.Span[..bytesRead]);
    }

    return totalRead;
}

Span\<T> is a hot-path tool — not for general use

Span<T> cannot be used as a field in a class, across await boundaries, or in async methods. Use it for CPU-bound processing in tight loops, parser hot paths, or format operations where allocation profiling has shown a measurable problem.

`IAsyncEnumerable<T>` for Streaming Results

Use IAsyncEnumerable<T> when you need to stream a large sequence without loading it all into memory first:

// ✅ IAsyncEnumerable — stream results without buffering all rows
public async IAsyncEnumerable<Order> StreamOrdersAsync(
    int userId,
    [EnumeratorCancellation] CancellationToken ct = default)
{
    await foreach (var order in _context.Orders
        .Where(o => o.UserId == userId)
        .AsNoTracking()
        .AsAsyncEnumerable()
        .WithCancellation(ct))
    {
        yield return order;
    }
}

// ✅ Consumer — processes each item as it arrives, no full materialisation
await foreach (var order in orderService.StreamOrdersAsync(userId, ct))
{
    await exportService.WriteLineAsync(order, ct);
}

// ❌ Loading all records into memory — problematic for large datasets
var orders = await _context.Orders
    .Where(o => o.UserId == userId)
    .ToListAsync(ct);   // 100k records → 100k objects in memory

SOLID Principles in Practice

Single Responsibility

Each class should have one reason to change:

// ❌ Controller doing too much — validation, business logic, persistence, email
[HttpPost]
public async Task<IActionResult> CreateOrder([FromBody] CreateOrderRequest request)
{
    if (request.Items.Count == 0) return BadRequest("No items");
    decimal total = request.Items.Sum(i => i.Price * i.Quantity);
    var order = new Order { UserId = request.UserId, Total = total };
    _context.Orders.Add(order);
    await _context.SaveChangesAsync();
    await _emailClient.SendAsync(request.Email, "Order confirmed");
    return Ok(order);
}

// ✅ Controller delegates to service — one responsibility each
[HttpPost]
public async Task<IActionResult> CreateOrder(
    [FromBody] CreateOrderRequest request,
    CancellationToken ct)
{
    var order = await _orderService.CreateOrderAsync(request, ct);
    return CreatedAtAction(nameof(GetOrder), new { id = order.Id }, order.ToResponse());
}

Dependency Inversion

Depend on abstractions (interface), not implementations:

// ✅ Depends on IOrderRepository — can be mocked in tests
public sealed class OrderService
{
    private readonly IOrderRepository _repository;
    public OrderService(IOrderRepository repository) => _repository = repository;
}

// ❌ Depends on concrete class — cannot be mocked, tightly coupled
public class OrderService
{
    private readonly SqlOrderRepository _repository;
    public OrderService() => _repository = new SqlOrderRepository();  // new = tight coupling
}

Logging

Use ILogger<T> with structured message templates — curly-brace placeholders produce indexed log properties that are searchable in log aggregation tools (Datadog, Seq, ELK).

// ✅ Structured logging — properties are indexed and searchable
_logger.LogInformation(
    "Order {OrderId} created for user {UserId}. Total: {Total} {Currency}",
    order.Id, order.UserId, order.Total, order.Currency);

_logger.LogWarning(
    "Payment declined for order {OrderId}: decline code {DeclineCode}",
    order.Id, result.DeclineCode);

_logger.LogError(
    ex,
    "Unhandled exception processing order {OrderId}",
    orderId);

// ❌ String interpolation in log calls — defeats structured logging
_logger.LogInformation($"Order {order.Id} created for user {order.UserId}");
// ❌ Console.WriteLine — never in production code
Console.WriteLine("Order created");

Log Level Guidelines

Level	Use When
`LogTrace`	Very detailed — only for active debugging sessions, never in production
`LogDebug`	Diagnostic detail — disabled by default in production
`LogInformation`	Key business events — order created, payment succeeded
`LogWarning`	Unexpected but handled — payment declined, retry attempt, validation failure
`LogError`	Failures requiring attention — exceptions, operation failures
`LogCritical`	System-level failures — database down, service unresponsive

Comments

Implementation Comments

// ✅ Explains the business rule — not obvious from the code
// Stripe's idempotency key is limited to 255 chars. We prefix with the order ID
// and timestamp to guarantee uniqueness across retries.
var idempotencyKey = $"order_{orderId}_{DateTimeOffset.UtcNow.ToUnixTimeMilliseconds()}";

// ✅ Explains a non-obvious performance decision
// HashSet for O(1) lookup — product catalogue has ~50k items and this runs per request.
var productIdSet = new HashSet<int>(catalogue.Select(p => p.Id));

// ❌ Narrates the code — zero information
// Increment the counter
count++;

// ❌ Commented-out code — delete it; git history preserves it
// var oldResult = LegacyProcessOrder(order);

Special Markers

// TODO(PROJ-1234): Replace with async notification when event bus is available.
await emailService.SendSyncAsync(user.Email, message, ct);

// FIXME(PROJ-5678): Refund calculation does not account for partial returns.
decimal refundTotal = order.Total;

// HACK: Stripe v3 SDK does not support idempotency on capture. Remove after upgrade.
await _stripeClient.CaptureWithoutIdempotencyAsync(chargeId, ct);

Markers must reference a ticket

A TODO or FIXME without a ticket is invisible to project planning. Always include the ticket ID.

Testing

Test File Naming

OrderService.cs              →  OrderServiceTests.cs
PaymentProcessor.cs          →  PaymentProcessorTests.cs
UserRepository.cs            →  UserRepositoryIntegrationTests.cs
OrderController.cs           →  OrderControllerTests.cs

Test Structure — Arrange / Act / Assert

using FluentAssertions;
using NSubstitute;
using Xunit;

public class OrderServiceTests
{
    private readonly IOrderRepository _repository = Substitute.For<IOrderRepository>();
    private readonly IPaymentService _paymentService = Substitute.For<IPaymentService>();
    private readonly ILogger<OrderService> _logger = Substitute.For<ILogger<OrderService>>();
    private readonly OrderService _sut;

    public OrderServiceTests()
    {
        _sut = new OrderService(_repository, _paymentService, _logger);
    }

    [Fact]
    public async Task GetOrderAsync_WhenOrderExists_ReturnsOrder()
    {
        // Arrange
        var expected = new Order { Id = 1, UserId = 42, Total = 99.99m };
        _repository.FindByIdAsync(1, Arg.Any<CancellationToken>())
                   .Returns(expected);

        // Act
        var result = await _sut.GetOrderAsync(1);

        // Assert
        result.Should().BeEquivalentTo(expected);
        await _repository.Received(1).FindByIdAsync(1, Arg.Any<CancellationToken>());
    }

    [Fact]
    public async Task GetOrderAsync_WhenOrderNotFound_ThrowsOrderNotFoundException()
    {
        // Arrange
        _repository.FindByIdAsync(999, Arg.Any<CancellationToken>())
                   .Returns((Order?)null);

        // Act
        var act = () => _sut.GetOrderAsync(999);

        // Assert
        await act.Should().ThrowAsync<OrderNotFoundException>()
            .Where(ex => ex.OrderId == 999);
    }

    [Theory]
    [InlineData(0)]
    [InlineData(-1)]
    [InlineData(-100)]
    public async Task GetOrderAsync_WhenIdIsInvalid_ThrowsArgumentOutOfRangeException(int id)
    {
        // Act
        var act = () => _sut.GetOrderAsync(id);

        // Assert
        await act.Should().ThrowAsync<ArgumentOutOfRangeException>()
            .WithParameterName("orderId");
    }
}

Recommended test libraries:

Library	Purpose
xUnit	Test framework
FluentAssertions	Readable assertion syntax
NSubstitute	Mocking
Bogus	Generating realistic test data
WebApplicationFactory	Integration tests for ASP.NET Core
Respawn	Database reset between integration tests

Coverage expectations:

Layer	Target
Service layer business logic	90%+ unit test coverage
Domain models and value objects	100%
Repository layer	Integration tests with a test database
API controllers	Integration tests covering all response codes

Quick Reference

Practice	Rule
XML docs	All `public`/`protected` members must be documented
Exceptions	Specific types; typed custom exceptions with identifier properties; global handler at API boundary
DI	Constructor injection; `readonly` fields; always depend on interfaces
Options pattern	`IOptions<T>` for config — never inject `IConfiguration` directly into services
sealed	All concrete leaf classes are `sealed` by default
IDisposable	Implement when owning disposable members; prefer `using` declarations
EF Core	`AsNoTracking()` for reads; `Select` for projection; `ExecuteDeleteAsync`/`ExecuteUpdateAsync` for bulk ops
Logging	`ILogger<T>` with structured message templates; never `Console.WriteLine`
async	All I/O is async; always pass `CancellationToken`; no `.Result`/`.Wait()`
Nulls	Enable `<Nullable>enable</Nullable>`; use `??`, `?.`, `??=`; validate at public boundaries
SOLID	One reason per class; depend on interfaces; inject all dependencies
Testing	xUnit + FluentAssertions + NSubstitute; Arrange/Act/Assert; 90%+ service coverage
Comments	Explain why, not what; TODO/FIXME must include ticket ID
Span\<T>	Zero-allocation slicing in hot paths — not for general use
IAsyncEnumerable\<T>	Streaming large sequences — avoids full materialisation