Clean C# in Production: Design Patterns and Defensive Code

Let’s treat this as a design review rather than a pattern quiz.

When I mentor a developer on design patterns, I do not begin by asking them to recite Factory, Strategy or Decorator definitions. I begin with a change request. A lender has one affordability rule today, three tomorrow, and a partner-specific exception next month. The approval endpoint must be secure, auditable and safe to retry. An external credit provider is unreliable. Operations needs useful diagnostics without customer data leaking into logs.

That is where design becomes real.

Clean code and design patterns solve related but different problems. Clean code helps the next developer understand a method, class or module. Patterns help a team recognise a recurring design pressure and discuss a proven shape for handling it. Defensive code asks what happens when input is hostile, a dependency fails, two requests race, cancellation arrives or a valid business state changes beneath us.

My central message is simple: patterns are not decorations. A pattern earns its place when it removes a specific source of change, coupling or risk. If the pattern makes the code harder to understand than the problem, it is the wrong design—or it has arrived too early.

Start with the problem, not the pattern

Imagine a loan platform with this use case:

Receive application
Validate input
Load customer and product rules
Request a credit assessment
Calculate affordability
Decide whether manual review is required
Persist the decision
Publish an audit event
Return a safe API response

The tempting approach is one LoanService with database calls, HTTP calls, calculations, logging and email logic. It works during the first sprint. Then requirements change and every edit touches the same class. Tests need five mocks. A retry sends two emails. A timeout leaves the database in an uncertain state. Developers become frightened of changing it.

Before choosing a pattern, I ask four questions:

1. What changes independently?
2. What must remain invariant?
3. Where does the outside world enter the design?
4. Which failure would be expensive or unsafe?

The affordability algorithm changes independently from credit-provider integration. A submitted application must keep valid monetary and identity data. HTTP, clocks, databases and message brokers are outside-world boundaries. Duplicate approvals, leaked data and inconsistent decisions are expensive failures.

Those answers give us design seams. Patterns may then help us name and implement them.

Clean code is the admission price

A pattern cannot rescue unreadable code. Before introducing abstractions, make the ordinary code honest.

Names should expose intent. Compare this:

public bool Check(decimal a, decimal b, int t)
    => a / b < t;

with this:

public bool IsWithinLoanToIncomeLimit(
    decimal requestedAmount,
    decimal annualIncome,
    decimal maximumRatio)
{
    if (annualIncome <= 0)
        return false;

    return requestedAmount / annualIncome <= maximumRatio;
}

The longer version is easier to review because its concepts are visible. Brevity is not the same as clarity.

Keep methods at one level of abstraction. A method called ApproveAsync should describe the approval workflow; it should not also contain raw SQL, JSON parsing and SMTP configuration. Extract code when the extracted name explains a business step, not simply to achieve an arbitrary line count.

Comments should explain a surprising constraint or decision. They should not translate C# into English:

// Add one to retry count
retryCount++;

A useful comment preserves knowledge:

// The provider may accept a request after our timeout. Reuse the same
// idempotency key so a retry cannot create a second credit search.

Consistency matters because every style variation consumes attention. Use formatters, analyzers, nullable reference types and warnings as errors where practical. Automation should settle formatting debates so reviews can focus on correctness, behaviour, security and design.

Build a domain model that protects itself

An anemic model lets any caller create impossible state and expects services to remember every rule. A defensive model makes invalid construction difficult.

public sealed class LoanApplication
{
    public Guid Id { get; }
    public ApplicantId ApplicantId { get; }
    public Money RequestedAmount { get; }
    public LoanStatus Status { get; private set; }
    public DateTimeOffset SubmittedAt { get; private set; }

    private LoanApplication(
        Guid id,
        ApplicantId applicantId,
        Money requestedAmount)
    {
        Id = id;
        ApplicantId = applicantId;
        RequestedAmount = requestedAmount;
        Status = LoanStatus.Draft;
    }

    public static LoanApplication Create(
        ApplicantId applicantId,
        Money requestedAmount)
    {
        if (requestedAmount.Amount <= 0)
            throw new DomainRuleException("Requested amount must be positive.");

        return new LoanApplication(Guid.NewGuid(), applicantId, requestedAmount);
    }

    public void Submit(TimeProvider timeProvider)
    {
        if (Status != LoanStatus.Draft)
            throw new DomainRuleException("Only draft applications can be submitted.");

        Status = LoanStatus.Submitted;
        SubmittedAt = timeProvider.GetUtcNow();
    }
}

Notice the defensive choices. Setters are not public. Creation is named. State transitions guard their preconditions. Time arrives through TimeProvider, so tests do not depend on the wall clock. The entity owns rules that are intrinsic to its valid lifecycle.

Do not push every rule into an entity. A rule requiring live exchange rates or a customer’s complete portfolio probably belongs in a domain service. The boundary is intent: an entity protects its own invariants; an application service coordinates a use case; infrastructure talks to external systems.

Value objects improve the model further. decimal alone cannot tell us whether a value is GBP or USD. A Money value object can validate currency, rounding and arithmetic once. This is not ceremony when monetary correctness matters.

Strategy: isolate a rule that genuinely varies

Strategy is useful when several algorithms fulfil the same business purpose and selection can change independently from the caller.

public interface IAffordabilityStrategy
{
    AffordabilityResult Assess(AffordabilityContext context);
}

public sealed class StandardAffordabilityStrategy : IAffordabilityStrategy
{
    public AffordabilityResult Assess(AffordabilityContext context)
    {
        if (context.AnnualIncome <= 0)
            return AffordabilityResult.Reject("Annual income is required.");

        var ratio = context.RequestedAmount / context.AnnualIncome;
        return ratio <= 4.5m
            ? AffordabilityResult.Accept(ratio)
            : AffordabilityResult.Refer(ratio, "Ratio exceeds standard limit.");
    }
}

public sealed class BridgingAffordabilityStrategy : IAffordabilityStrategy
{
    public AffordabilityResult Assess(AffordabilityContext context)
    {
        if (context.ExitValue is null || context.ExitValue <= 0)
            return AffordabilityResult.Reject("A valid exit value is required.");

        var loanToValue = context.RequestedAmount / context.ExitValue.Value;
        return loanToValue <= 0.70m
            ? AffordabilityResult.Accept(loanToValue)
            : AffordabilityResult.Refer(loanToValue, "LTV exceeds bridging limit.");
    }
}

The calling workflow depends on IAffordabilityStrategy, not a giant switch. This supports the Open/Closed Principle: add a product strategy without rewriting the approval workflow.

The gotcha is fragmentation. If two strategies differ by one constant, two classes may hide the real rule. A configuration-driven policy could be clearer. Strategy is justified when behaviour varies, not merely data.

Also be careful about selecting a strategy from untrusted input. A request should not provide a .NET type name. Map a validated product code to a registered strategy. Unknown codes should fail explicitly, not silently fall back to a default that produces the wrong lending decision.

Factory: centralise construction decisions

Factory is valuable when object creation involves selection, configuration or validation that should not leak across consumers.

public sealed class AffordabilityStrategyFactory
{
    private readonly IReadOnlyDictionary<LoanProductType, IAffordabilityStrategy> _strategies;

    public AffordabilityStrategyFactory(IEnumerable<IAffordabilityStrategy> strategies)
    {
        _strategies = strategies.ToDictionary(GetProductType);
    }

    public IAffordabilityStrategy Create(LoanProductType productType)
        => _strategies.TryGetValue(productType, out var strategy)
            ? strategy
            : throw new NotSupportedException(
                $"No affordability strategy is registered for {productType}.");

    private static LoanProductType GetProductType(IAffordabilityStrategy strategy)
        => strategy switch
        {
            StandardAffordabilityStrategy => LoanProductType.Standard,
            BridgingAffordabilityStrategy => LoanProductType.Bridging,
            _ => throw new InvalidOperationException(
                $"Strategy {strategy.GetType().Name} has no product mapping.")
        };
}

In a larger system I might use keyed dependency injection or explicit metadata rather than the type switch, but the principle remains: construction knowledge has one home.

Factories are often overused. new CustomerName(value) does not need a CustomerNameFactory when the constructor already expresses the complete creation rule. A factory should remove meaningful creation complexity, not conceal a simple constructor.

Do not turn the factory into a service locator that resolves arbitrary dependencies at runtime. Service location hides a class’s real dependencies and moves errors from startup to execution. Constructor injection remains the default; a factory handles a specific family of creation decisions.

Decorator: add behaviour without contaminating the core

Logging, caching, metrics, retries and authorisation often surround a use case. Putting all of them in the core handler makes the business flow unreadable. Decorator wraps an implementation behind the same interface.

public interface ICreditAssessmentGateway
{
    Task<CreditAssessment> AssessAsync(
        CreditAssessmentRequest request,
        CancellationToken cancellationToken);
}

public sealed class LoggingCreditAssessmentGateway(
    ICreditAssessmentGateway inner,
    ILogger<LoggingCreditAssessmentGateway> logger)
    : ICreditAssessmentGateway
{
    public async Task<CreditAssessment> AssessAsync(
        CreditAssessmentRequest request,
        CancellationToken cancellationToken)
    {
        using var scope = logger.BeginScope(new Dictionary<string, object>
        {
            ["ApplicationId"] = request.ApplicationId,
            ["Provider"] = request.ProviderCode
        });

        var started = Stopwatch.GetTimestamp();
        try
        {
            var result = await inner.AssessAsync(request, cancellationToken);
            logger.LogInformation(
                "Credit assessment completed in {ElapsedMs} ms with {Outcome}",
                Stopwatch.GetElapsedTime(started).TotalMilliseconds,
                result.Outcome);
            return result;
        }
        catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
        {
            logger.LogInformation("Credit assessment was cancelled by the caller");
            throw;
        }
    }
}

The decorator does not log names, addresses or raw credit responses. Structured identifiers are enough for correlation. Defensive logging treats logs as another data store subject to privacy and retention controls.

Order matters when decorators are stacked. Retry outside a metrics decorator measures every attempt; metrics outside retry measures the complete operation. Caching outside authorisation can leak data if the cache key ignores user or tenant boundaries. Document the order and test it.

Avoid blind retries. Retry transient failures such as a short network interruption, not validation errors or every 500. A non-idempotent operation needs a stable idempotency key before retry can be safe. Add jitter and bounded attempts so thousands of instances do not retry simultaneously.

Facade and Adapter: protect the domain from external complexity

Third-party APIs rarely speak the language of our domain. They expose provider codes, awkward DTOs and failure semantics we do not want everywhere. Adapter translates one contract into another. Facade presents a simple operation over several complicated steps.

public sealed class CreditAssessmentFacade(
    IIdentityVerificationClient identityClient,
    ICreditBureauClient bureauClient,
    IRiskScoreCalculator calculator)
{
    public async Task<CreditAssessment> AssessAsync(
        LoanApplication application,
        CancellationToken cancellationToken)
    {
        var identity = await identityClient.VerifyAsync(
            application.ApplicantId,
            cancellationToken);

        if (!identity.IsVerified)
            return CreditAssessment.Rejected("Identity could not be verified.");

        var bureauReport = await bureauClient.GetReportAsync(
            application.ApplicantId,
            cancellationToken);

        return calculator.Calculate(identity, bureauReport);
    }
}

The rest of the application does not learn how a bureau represents missing history or which endpoint performs identity matching. Translation happens at the boundary.

Defensive adapters validate external responses. A successful HTTP status does not guarantee a complete payload. Check required fields, ranges and enum values. Map unknown provider values to an explicit Unknown, quarantine the response or fail safely. Never reinterpret an unrecognised risk category as low risk.

Keep provider DTOs in infrastructure. If generated classes spread through the domain, a vendor schema change becomes an application-wide refactor. The adapter is an anti-corruption layer: it protects our model and vocabulary.

Command: represent an auditable intention

Command is useful when an operation needs validation, authorisation, logging, queueing or retry as a first-class unit.

public sealed record ApproveLoanCommand(
    Guid ApplicationId,
    string ExpectedVersion,
    string IdempotencyKey,
    UserId ApprovedBy);

public sealed class ApproveLoanHandler(
    ILoanRepository repository,
    IUnitOfWork unitOfWork,
    IOutbox outbox,
    TimeProvider timeProvider)
{
    public async Task<ApproveLoanResult> HandleAsync(
        ApproveLoanCommand command,
        CancellationToken cancellationToken)
    {
        var priorResult = await repository.FindResultByIdempotencyKeyAsync(
            command.IdempotencyKey,
            cancellationToken);

        if (priorResult is not null)
            return priorResult;

        var loan = await repository.GetForUpdateAsync(
            command.ApplicationId,
            cancellationToken)
            ?? throw new LoanNotFoundException(command.ApplicationId);

        loan.Approve(command.ApprovedBy, timeProvider.GetUtcNow());

        repository.RequireVersion(command.ExpectedVersion);
        outbox.Add(new LoanApprovedIntegrationEvent(loan.Id));

        await unitOfWork.SaveChangesAsync(cancellationToken);
        return ApproveLoanResult.From(loan);
    }
}

This example combines several production safeguards. The command captures intent and caller context. The idempotency key makes a network retry safe. Optimistic concurrency prevents a stale approval from overwriting a newer decision. The outbox stores the integration event in the same transaction as the state change, avoiding the classic failure where the database commits but publishing fails.

The handler still needs authorisation before the state transition. Do not trust a user ID simply because it appears in the request body. Derive identity and claims from the authenticated principal, then create the command inside the trusted application boundary.

Command does not require a mediator library. A library may provide useful pipelines, but the pattern is the separation of an intention from its execution. Do not create a command, handler and six interfaces for trivial local operations if the indirection brings no benefit.

Observer and events: decouple reactions carefully

After approval, several things may happen: update reporting, notify the broker, create an audit entry and prepare documents. Domain events allow the aggregate to announce a meaningful fact without calling every consumer directly.

public sealed record LoanApprovedDomainEvent(
    Guid ApplicationId,
    UserId ApprovedBy,
    DateTimeOffset ApprovedAt);

The name is past tense because the event describes something that happened. Consumers should not reinterpret it as a request that might be rejected.

In-process events are simple but share the caller’s fate. If an email handler throws, should the approval transaction fail? Often the answer is no. Integration events through an outbox and broker give durability and independent retries, but introduce eventual consistency, duplicate delivery and operational complexity.

Every event consumer should be idempotent. Message brokers commonly guarantee at-least-once delivery, which means duplicates are normal. Store a processed-message identifier or design updates that can safely repeat.

Version event contracts. Removing a field can break a consumer deployed on a different schedule. Prefer additive evolution, clear ownership and contract tests. Never place secrets or unnecessary personal data on a broadly accessible message bus.

Observer becomes dangerous when it hides essential workflow. If five invisible handlers must succeed before an application is truly approved, model that workflow explicitly. Events are excellent for decoupled reactions; they are a poor disguise for a distributed transaction.

Builder: make complex valid objects readable

Builder helps when construction has many optional parts, must occur in stages or benefits from a readable test language.

public sealed class LoanApplicationBuilder
{
    private decimal _amount = 100_000m;
    private decimal _income = 50_000m;
    private LoanProductType _product = LoanProductType.Standard;

    public LoanApplicationBuilder WithAmount(decimal amount)
    {
        _amount = amount;
        return this;
    }

    public LoanApplicationBuilder WithIncome(decimal income)
    {
        _income = income;
        return this;
    }

    public LoanApplicationBuilder AsBridgingLoan()
    {
        _product = LoanProductType.Bridging;
        return this;
    }

    public LoanApplication Build()
        => LoanApplication.CreateForAssessment(_amount, _income, _product);
}

Tests become expressive:

var application = new LoanApplicationBuilder()
    .AsBridgingLoan()
    .WithAmount(600_000m)
    .WithIncome(90_000m)
    .Build();

The builder should still call production validation. A test builder that bypasses invariants creates impossible fixtures and gives false confidence. Sensible defaults reduce noise, while explicit methods highlight what matters to each test.

In production, consider whether named factory methods or parameter objects are simpler. Builder is not automatically appropriate when a constructor has three arguments. Its value is readable, controlled assembly—not fashionable fluency.

Structural patterns: prefer composition over subclass explosions

Decorator, Facade and Adapter are common because integrations and cross-cutting behaviour are common. Composite and Bridge solve more specific pressures.

Composite gives individual objects and groups a common interface. In a fee calculation, an individual fee and a bundle of fees can both implement IFeeComponent. The bundle recursively sums its children. This is a natural fit for tree-shaped data such as menus, organisational structures or product bundles.

Bridge separates two dimensions that must vary independently. Suppose loan documents vary by product and rendering format. Subclassing every combination produces BridgingPdfDocument, BridgingHtmlDocument, CommercialPdfDocument and so on. Bridge composes a document definition with a renderer so products and formats grow separately.

The gotcha is premature abstraction. If there is one product and one output format, a bridge is a prediction, not a response to evidence. YAGNI matters. Introduce the seam when requirements or a clear roadmap show two independent dimensions.

Composition is not automatically clean. A graph of tiny objects can be harder to navigate than one cohesive class. High cohesion is the objective: code that changes for the same reason should usually live together. Low coupling means a module knows as little as practical about unrelated modules. Patterns help when they improve those qualities.

Exceptions: communicate failure without losing meaning

Exceptions should represent exceptional failure, not ordinary control flow. A search returning no records may be a normal empty result. Loading a required aggregate for approval and finding nothing is a meaningful failure.

Create a small, useful exception vocabulary:

public abstract class DomainException(string message) : Exception(message);

public sealed class DomainRuleException(string message)
    : DomainException(message);

public sealed class LoanNotFoundException(Guid id)
    : DomainException($"Loan application {id} was not found.");

public sealed class CreditProviderUnavailableException(
    string provider,
    Exception innerException)
    : Exception($"Credit provider {provider} is unavailable.", innerException);

Catch an exception only when you can recover, add meaningful context or translate it at a boundary. Never do this:

catch (Exception)
{
    return null;
}

It erases the failure and converts every downstream null reference into a detective story.

At the API boundary, map known exceptions to safe Problem Details responses. A domain-rule violation may become 400 or 409; missing data may become 404; an unexpected exception becomes 500 with a correlation ID. Do not return stack traces, SQL details or provider credentials.

Preserve the original exception when wrapping. Use throw; rather than throw ex; to retain the stack trace. Treat cancellation separately: an OperationCanceledException caused by the caller is not an infrastructure outage and should not be logged as an error.

Nulls, collections and boundary validation

Enable nullable reference types and take warnings seriously. They do not eliminate nulls, but they force intent into signatures.

Validate at trust boundaries. An API request is untrusted even when a frontend normally generates it. Check shape, length, range and cross-field rules. Normalise cautiously: trimming a name may be acceptable; silently changing an account identifier may not be.

public sealed record CreateLoanRequest(
    string ApplicantReference,
    decimal RequestedAmount,
    string Currency);

public static Result<ValidatedLoanRequest> Validate(CreateLoanRequest? request)
{
    if (request is null)
        return Result.Invalid("Request body is required.");

    if (string.IsNullOrWhiteSpace(request.ApplicantReference))
        return Result.Invalid("Applicant reference is required.");

    if (request.ApplicantReference.Length > 64)
        return Result.Invalid("Applicant reference is too long.");

    if (request.RequestedAmount is <= 0 or > 50_000_000m)
        return Result.Invalid("Requested amount is outside the supported range.");

    if (!SupportedCurrencies.Contains(request.Currency))
        return Result.Invalid("Currency is not supported.");

    return Result.Valid(new ValidatedLoanRequest(
        request.ApplicantReference.Trim(),
        request.RequestedAmount,
        request.Currency.ToUpperInvariant()));
}

Return empty collections rather than null when “no items” is valid. Expose IReadOnlyList when callers should not mutate the collection. Make defensive copies when accepting mutable collections across a boundary.

Do not validate only in the controller. Domain invariants must remain protected when the same use case is invoked by a queue consumer, scheduled job or test. Boundary validation improves feedback; domain validation preserves correctness.

Async, cancellation and concurrency gotchas

Async improves scalability for waiting work; it does not make CPU-bound work faster. Keep async flows async from endpoint to database or HTTP client. Avoid .Result and .Wait(), which can block threads and create deadlocks in some environments.

Pass CancellationToken through every meaningful asynchronous boundary. Cancellation is cooperative: code must forward and observe it. Do not replace the caller’s token with CancellationToken.None because cancellation is inconvenient.

Use timeouts as well as cancellation. A caller may never cancel. HTTP clients should be created through IHttpClientFactory, with resilience policies appropriate to the operation. Reuse connections; do not create and dispose HttpClient for each request.

Concurrency makes check-then-act code unsafe:

if (loan.Status == LoanStatus.Submitted)
{
    loan.Approve();
    await db.SaveChangesAsync(cancellationToken);
}

Two requests can both observe Submitted. Use optimistic concurrency with a row version, a database constraint or an atomic update. Catch the specific concurrency exception and return a conflict that invites the client to reload.

Avoid holding database transactions open while calling remote services. The network can stall, increasing lock time and reducing throughput. Separate the workflow, persist a pending state, or use messages and compensating actions where necessary. Distributed systems do not gain atomicity because we wrapped local code in TransactionScope.

Design APIs as contracts, not controller methods

An API contract outlives its implementation. Use resource-oriented routes, correct status codes and stable response shapes. Document errors as carefully as success.

app.MapPost("/api/loans/{id:guid}/approval", async (
    Guid id,
    ApproveLoanRequest request,
    ClaimsPrincipal user,
    IApproveLoanUseCase useCase,
    CancellationToken cancellationToken) =>
{
    var approverId = UserId.FromClaims(user);
    var command = new ApproveLoanCommand(
        id,
        request.ExpectedVersion,
        request.IdempotencyKey,
        approverId);

    var result = await useCase.ExecuteAsync(command, cancellationToken);

    return Results.Ok(new ApproveLoanResponse(
        result.ApplicationId,
        result.Status,
        result.Version));
})
.RequireAuthorization("LoanApprover")
.Produces<ApproveLoanResponse>(StatusCodes.Status200OK)
.ProducesProblem(StatusCodes.Status404NotFound)
.ProducesProblem(StatusCodes.Status409Conflict);

Do not expose EF Core entities directly. Persistence navigation properties, internal fields and future schema changes should not define the public contract. Map to explicit request and response DTOs.

Protect mass-assignment boundaries. If a request model includes Status, ApprovedBy or IsAdmin, a client may try to set them. Accept only fields the caller is allowed to control.

Version deliberately. Additive changes are usually safer than renaming or removing fields. Publish OpenAPI, include representative examples, and run contract checks in CI. Documentation is part of the feature, not a task for later.

Testing patterns and behaviour

Tests should protect behaviour and design decisions, not mirror implementation line by line. Start with domain tests because they are fast and precise.

[Fact]
public void Standard_strategy_refers_when_ratio_exceeds_limit()
{
    var strategy = new StandardAffordabilityStrategy();
    var context = new AffordabilityContext(
        RequestedAmount: 300_000m,
        AnnualIncome: 50_000m,
        ExitValue: null);

    var result = strategy.Assess(context);

    result.Outcome.Should().Be(AffordabilityOutcome.ManualReview);
    result.Reason.Should().Contain("standard limit");
}

Add boundary tests for invalid and extreme monetary values. Test that builders cannot create invalid objects. Test decorator order where it affects security or metrics. Test an adapter with captured provider payloads, including missing and unknown fields.

Integration tests should verify database mappings, transactions, concurrency and API status codes. A unit test mocking DbSet does not prove that a LINQ query translates to SQL. Use the real database engine in a disposable test environment when provider behaviour matters.

Contract tests protect external integrations. Verify the requests we send and responses we accept. Resilience tests should simulate timeouts, transient failure and duplicate delivery. Security tests should prove that one tenant cannot retrieve another tenant’s data.

Avoid overspecified mocks. A test that asserts every internal method call becomes brittle during harmless refactoring. Assert observable outcomes and essential collaborations. If a class needs ten mocks, pause: it may have too many responsibilities.

Code reviews: review the system, not only the syntax

A useful review asks more than whether the code compiles. I look at two dimensions: design and behaviour.

For design:

• Does this fit the existing architecture and business vocabulary?
• Is the new abstraction responding to real variation?
• Are responsibilities cohesive and dependencies explicit?
• Is a pattern helping, or merely making the solution look sophisticated?
• Can code be removed rather than added?
• Is the change easy to test and observe?

For behaviour:

• What happens with invalid, missing or duplicated input?
• What happens when a dependency times out?
• Is the operation safe to retry?
• Can concurrent requests corrupt state?
• Are security and tenant boundaries preserved?
• Do logs help investigation without leaking sensitive data?
• Is rollback safe, and are migrations compatible with mixed versions?

Reviews should be respectful and evidence based. Explain the risk behind a suggestion. “Use Strategy” is weak feedback. “This product-specific calculation changes independently and the conditional has grown in three releases; separating algorithms would let us test and add products without editing the approval workflow” is actionable.

Do not make a pull request the first moment anybody discusses the design. For consequential work, sketch the flow, boundaries and failure cases before coding. A small sequence diagram or decision record can prevent days of refactoring.

Anti-patterns and common mistakes

The Golden Hammer appears when a developer learns one pattern and sees it everywhere. Not every object needs a factory. Not every operation needs a command bus. Not every conditional needs Strategy. Familiarity is not evidence of suitability.

Singleton is a frequent trap. Global mutable state hides dependencies, complicates tests and creates concurrency problems. In ASP.NET Core, a singleton service also lives across requests; it must be thread-safe and must not capture scoped dependencies. Use the container’s lifetime management deliberately, not a hand-written global Instance property.

Repository can become an unnecessary wrapper over EF Core when it exposes generic CRUD and removes useful query capabilities. A repository earns its place when it expresses aggregate or domain operations, protects persistence boundaries or enables a meaningful alternative implementation.

DRY is often misread. Two similar lines are not automatically the same knowledge. Prematurely merging them can couple features that will evolve differently. Remove duplication when it represents the same rule and must change together.

Inheritance is commonly used for reuse when composition is safer. Deep hierarchies make behaviour depend on distant base classes and fragile overrides. Use inheritance for a genuine substitutable “is-a” relationship; use composition to assemble capabilities.

Other warning signs include catch-all exceptions, boolean parameters that change a method’s personality, hidden service location, enormous DTOs, comments apologising for confusing code, fire-and-forget tasks, unbounded parallelism, retries without idempotency, and abstractions with only one trivial implementation and no clear pressure for another.

Documentation is part of the design

Documentation should reduce the cost of making a correct change. It should not repeat every type and method name that an IDE can already show.

Begin with a short architectural overview. Explain the business capability, the major boundaries and the direction of dependencies. A new developer should be able to answer: where does a loan decision begin, where are product rules implemented, how do we reach external credit providers, and how is an approval event delivered?

Use a small diagram when relationships matter:

API endpoint
    |
    v
Approval use case ---> Strategy factory ---> Product strategy
    |
    +---> Credit facade ---> Provider adapter ---> External API
    |
    +---> Repository / Unit of Work ---> SQL database
    |
    +---> Outbox ---> Message broker ---> Notifications / Reporting

The diagram does not need every class. Its job is to preserve the important decisions and help a reader navigate the code.

For a significant pattern, write a lightweight architecture decision record. Capture the context, decision, alternatives and consequences. For example:

Decision: Use Strategy for affordability rules.

Context:
Product calculations change independently and are owned by different
business teams. The existing conditional is modified by most releases.

Alternatives:
One conditional service; configuration-only rules; rules engine.

Consequences:
Each product has an isolated, testable algorithm. Selection must be
validated centrally. Cross-product changes may touch several strategies.

That final consequences section is important. Every pattern has a cost. Strategy adds types and navigation. Events add eventual consistency. Decorators make runtime order significant. A record that only advertises benefits is marketing, not engineering documentation.

Use XML documentation for public libraries where consumers need behaviour, parameter, return and exception guarantees. Document units and ranges. “Amount” is ambiguous; “amount in the currency’s major unit, greater than zero” is a contract. Say whether a method is idempotent, whether it may return cached data, which exceptions are stable, and whether cancellation can leave partial work.

OpenAPI should describe examples, validation rules, authentication, status codes and Problem Details responses. Operational runbooks should explain dashboards, alerts, retry or replay procedures, dependency failure modes and safe rollback. A design is incomplete if only its original author knows how to operate it.

Keep documentation near the artefact it describes and check links in CI. Delete obsolete documents. Incorrect documentation is more dangerous than missing documentation because it creates confident mistakes.

Make pattern selection an explicit decision

When a developer proposes a pattern, I ask them to complete five sentences:

The recurring problem is...
The part likely to vary is...
The pattern isolates that variation by...
The simpler alternative is...
We will know this design is failing when...

This prevents pattern-driven development. It also gives reviewers something concrete to challenge.

Suppose we consider a rules engine for affordability. The recurring problem is frequent rule change. The varying part is decision logic. The engine isolates rules from compiled application code. The simpler alternative is a typed Strategy per product. We know the engine is failing if rules become impossible to debug, version or test. That conversation may reveal that Strategy is enough today.

Consider event-driven approval. The recurring problem may be independent downstream reactions and slow integrations. Events isolate the approval transaction from those reactions. The simpler alternative is an in-process call. We know the design is failing if users cannot understand eventual status, duplicate processing creates side effects, or operations cannot trace a message across services.

Patterns should make change local. After implementing one, perform a thought experiment: add a new loan product, replace the credit provider, introduce tenant-specific logging and retry a duplicate approval. Count the modules that change. If the answer is still “nearly everything,” the abstraction may sit at the wrong boundary.

Also ask what should remain coupled. Splitting a cohesive business rule across a visitor, factory, strategy and mediator can make one concept impossible to read. Decoupling is not the goal by itself. We want strong cohesion inside a capability and deliberate coupling at stable contracts.

Finally, set a removal condition. If an abstraction no longer separates real variation, simplify it. Architecture is not a museum. Good teams remove patterns as readily as they introduce them when the underlying pressure disappears.

A practical refactoring path

Do not rewrite a working system into patterns in one heroic branch. Refactor through safe, observable steps.

First, characterise current behaviour with tests at stable boundaries. These tests are not approval of the design; they are a safety net.

Second, improve names and extract concepts without changing behaviour. Make dependencies explicit. Replace static clock and environment access with injectable boundaries where tests need control.

Third, identify the axis of change. If product algorithms vary, extract Strategy. If provider integration contaminates the domain, add an Adapter or Facade. If logging and resilience obscure business flow, introduce focused Decorators.

Fourth, move invariants into the domain model. Keep API and persistence models at their boundaries. Add explicit exception translation.

Fifth, introduce production safety: cancellation, timeout, idempotency, concurrency control, outbox delivery, structured logs and metrics. These are not polish; they are part of correctness.

Sixth, remove obsolete paths. A refactor that adds a new architecture but leaves the old one permanently doubles maintenance cost.

Measure the result. Useful measures include change failure rate, escaped defects, review time, test duration, incident diagnosis time and how many modules a normal feature touches. Line count alone is not a quality metric.

Mentoring workshop: review one approval change from request to production

The catalogue above is useful, but patterns become memorable when we follow a change through a real design. Imagine this request arrives on Monday:

Add a green-home-improvement loan. It uses a different affordability calculation, obtains an energy-rating check from a new partner, requires manual review when evidence is incomplete, and must not create two paid partner searches if the client retries.

The weak response is to add another if statement wherever the current product is checked. The equally weak “architectural” response is to create a pattern for every noun before understanding the flow. I would run a short design session with the junior developer.

Junior: This sounds like Strategy for the calculation, Factory for selection, Adapter for the partner and Command for approval. Shall I create those folders?

>

Senior: Those may be the resulting shapes. First show me the business states, the consistency boundary and the expensive failures. Folder names do not prove a design.

We sketch the happy path and the uncertain paths:

Submitted application
  -> validate product and evidence
  -> obtain or reuse energy assessment
  -> calculate affordability
  -> decide approved / referred / declined
  -> persist decision and outbox event atomically
  -> publish event at least once

Uncertainty:
  partner accepts request but our HTTP call times out
  application changes while assessment is running
  duplicate approval command arrives
  decision commits but broker is unavailable
  cancellation arrives after an external side effect

That diagram identifies three kinds of protection. Domain invariants keep an application valid. Integration controls make the paid search safe to retry. Persistence controls keep decision state and the event intent consistent. A Strategy only solves the varying calculation; it does not solve the other two.

Express the use case before distributing it across handlers

I prefer one readable application service that tells the story. Its collaborators hide technical detail but not the workflow:

public sealed class AssessGreenLoanUseCase(
    ILoanApplicationRepository applications,
    IAffordabilityStrategyResolver strategies,
    IEnergyAssessmentGateway energyAssessments,
    IUnitOfWork unitOfWork,
    ISystemClock clock,
    ILogger<AssessGreenLoanUseCase> logger)
{
    public async Task<AssessmentResult> ExecuteAsync(
        AssessGreenLoanCommand command,
        RequestIdentity actor,
        CancellationToken cancellationToken)
    {
        var application = await applications.LoadAsync(
            command.ApplicationId, actor.TenantId, cancellationToken)
            ?? throw new ApplicationNotFoundException(command.ApplicationId);

        application.EnsureVersion(command.ExpectedVersion);
        application.EnsureMayBeAssessedBy(actor);

        if (application.TryGetCompletedAssessment(command.IdempotencyKey, out var prior))
            return prior;

        var energy = await energyAssessments.GetOrCreateAsync(
            EnergyAssessmentRequest.From(application, command.IdempotencyKey),
            cancellationToken);

        var strategy = strategies.Resolve(application.Product);
        var decision = strategy.Assess(
            AffordabilityContext.From(application, energy, clock.UtcNow));

        application.RecordDecision(decision, energy.Reference, actor, clock.UtcNow,
            command.IdempotencyKey);
        application.AddDomainEvent(new LoanAssessmentCompleted(
            application.Id, decision.Outcome, application.Version));

        await unitOfWork.CommitAsync(cancellationToken);
        return AssessmentResult.From(application);
    }
}

This is orchestration code. It is allowed to name the steps. It does not calculate ratios, construct HTTP requests or publish directly to a broker. The method remains cohesive because all of its statements advance one use case.

Junior: Is the use case violating single responsibility because it loads, calls a gateway, calculates and saves?

>

Senior: Its responsibility is coordinating one business transaction. Single responsibility means one reason to change, not one statement per class. Splitting every line into a handler would hide the story without removing change.

The constructor has six dependencies, which is worth noticing but not automatically wrong. If the list grows, inspect whether the use case has absorbed unrelated concerns. Do not introduce a service locator to make the constructor look smaller; that merely hides dependencies.

Design the domain result as a value, not a boolean

An affordability method returning bool loses information. False could mean declined, incomplete evidence, provider unavailable or manual review. Model the decision explicitly:

public enum AssessmentOutcome
{
    Approved,
    ManualReview,
    Declined
}

public sealed record AssessmentDecision
{
    public AssessmentOutcome Outcome { get; }
    public string ReasonCode { get; }
    public IReadOnlyList<DecisionFactor> Factors { get; }

    private AssessmentDecision(
        AssessmentOutcome outcome,
        string reasonCode,
        IReadOnlyList<DecisionFactor> factors)
    {
        if (string.IsNullOrWhiteSpace(reasonCode))
            throw new ArgumentException("A stable reason code is required.");

        Outcome = outcome;
        ReasonCode = reasonCode;
        Factors = factors.ToArray();
    }

    public static AssessmentDecision Refer(
        string reasonCode, params DecisionFactor[] factors) =>
        new(AssessmentOutcome.ManualReview, reasonCode, factors);
}

Stable reason codes support policy, analytics and API mapping; user-facing text can be localised separately. Copying the factors prevents a caller from mutating the decision through a shared list. If factors contain sensitive details, define which projection may leave the domain.

Exceptions and outcomes should not be confused. “Income exceeds the permitted ratio” is an expected business outcome. “The strategy returned no decision” is a defect. “The partner timed out” is an operational failure or an explicit referral rule depending on agreed policy. Exceptions should represent inability to complete the promised operation, not ordinary branching.

Implement Strategy without creating a switch elsewhere

The green-loan strategy owns its calculation and evidence rules:

public sealed class GreenHomeAffordabilityStrategy : IAffordabilityStrategy
{
    public LoanProduct Product => LoanProduct.GreenHomeImprovement;

    public AssessmentDecision Assess(AffordabilityContext context)
    {
        ArgumentNullException.ThrowIfNull(context);

        if (context.EnergyAssessment is null)
            return AssessmentDecision.Refer("ENERGY_EVIDENCE_MISSING");

        if (context.AnnualIncome.Amount <= 0)
            return AssessmentDecision.Refer("INCOME_REQUIRES_REVIEW");

        var ratio = context.RequestedAmount.Amount / context.AnnualIncome.Amount;
        var permittedRatio = context.EnergyAssessment.Rating switch
        {
            EnergyRating.A or EnergyRating.B => 4.75m,
            EnergyRating.C => 4.25m,
            _ => 3.75m
        };

        return ratio <= permittedRatio
            ? AssessmentDecision.Approve("GREEN_RATIO_WITHIN_LIMIT",
                DecisionFactor.Decimal("loanToIncome", ratio))
            : AssessmentDecision.Refer("GREEN_RATIO_EXCEEDS_LIMIT",
                DecisionFactor.Decimal("loanToIncome", ratio));
    }
}

The figures are illustrative, not lending advice. In a real platform, product and compliance owners define them, effective dates are versioned, and calculations use an agreed rounding policy.

Selection can be registered once:

services.AddScoped<IAffordabilityStrategy, StandardAffordabilityStrategy>();
services.AddScoped<IAffordabilityStrategy, GreenHomeAffordabilityStrategy>();
services.AddScoped<IAffordabilityStrategyResolver, AffordabilityStrategyResolver>();

public sealed class AffordabilityStrategyResolver(
    IEnumerable<IAffordabilityStrategy> strategies)
    : IAffordabilityStrategyResolver
{
    private readonly IReadOnlyDictionary<LoanProduct, IAffordabilityStrategy> byProduct =
        strategies.ToDictionary(x => x.Product);

    public IAffordabilityStrategy Resolve(LoanProduct product) =>
        byProduct.TryGetValue(product, out var strategy)
            ? strategy
            : throw new UnsupportedLoanProductException(product);
}

Validate duplicate registrations at startup. Otherwise ToDictionary fails only when the resolver is first constructed, possibly on the first production request. A startup validation or focused container test provides earlier feedback.

Junior: The strategy contains a switch. Did Strategy not remove switches?

>

Senior: It localised product selection and keeps product calculations independent. A small exhaustive switch over energy ratings inside one algorithm is clear. Eliminating every conditional is not the goal.

If product rules are data maintained by authorised business users, configuration or a rules engine may eventually be justified. Start with typed code when rules change through releases and require developer review. A rules engine adds its own language, debugging, security, versioning and testing burden.

Build an anti-corruption Adapter around the partner

The partner calls a property an asset, returns single-letter statuses and uses a numeric score whose scale may change. Those details should stop at the adapter.

internal sealed class PartnerEnergyAssessmentAdapter(
    HttpClient httpClient,
    IPartnerTokenProvider tokens,
    ILogger<PartnerEnergyAssessmentAdapter> logger)
    : IEnergyAssessmentGateway
{
    public async Task<EnergyAssessment> GetOrCreateAsync(
        EnergyAssessmentRequest request,
        CancellationToken cancellationToken)
    {
        using var message = new HttpRequestMessage(HttpMethod.Post, "v2/assessments")
        {
            Content = JsonContent.Create(new PartnerAssessmentRequest(
                AssetReference: request.PropertyReference.Value,
                RequestedBand: request.RequiredBand.Code))
        };

        message.Headers.Add("Idempotency-Key", request.IdempotencyKey.Value);
        message.Headers.Authorization = new("Bearer",
            await tokens.GetAsync(cancellationToken));

        using var response = await httpClient.SendAsync(
            message, HttpCompletionOption.ResponseHeadersRead, cancellationToken);

        if (response.StatusCode == HttpStatusCode.Conflict)
            return await ResolveExistingAsync(request, cancellationToken);

        if (!response.IsSuccessStatusCode)
            throw PartnerFailureMapper.From(response.StatusCode);

        var payload = await response.Content.ReadFromJsonAsync<PartnerAssessmentResponse>(
            cancellationToken: cancellationToken)
            ?? throw new PartnerProtocolException("Partner returned an empty response.");

        return Map(payload);
    }
}

The example assumes the partner supports idempotency; verify the real contract. If it does not, our system may need a locally persisted operation record and a reconciliation lookup. A timeout does not prove failure. The request may have reached the partner and committed after our connection closed.

Do not log bearer tokens, full addresses or raw partner bodies. Log a correlation ID, safe property reference hash, operation key, duration, status category and partner request ID. Detailed payload capture, if legally permitted at all, belongs behind restricted diagnostic controls.

Mapping should be exhaustive and fail explicitly on unknown values:

private static EnergyAssessment Map(PartnerAssessmentResponse response) =>
    response.Status switch
    {
        "A" => EnergyAssessment.Complete(response.Id, MapRating(response.Score)),
        "P" => EnergyAssessment.Pending(response.Id),
        "N" => EnergyAssessment.NotFound(response.Id),
        _ => throw new PartnerProtocolException(
            $"Unknown partner status '{response.Status}'.")
    };

Silently treating a new status as “not found” keeps the API running but corrupts business meaning. The controlled exception should map to a safe dependency failure while an alert tells engineers the contract changed.

Put resilience in a Decorator, but respect semantics

Timeouts, tracing and metrics can wrap the gateway. Retry requires more judgement because it depends on operation semantics.

public sealed class TimedEnergyAssessmentGateway(
    IEnergyAssessmentGateway inner,
    TimeProvider time,
    IMetrics metrics) : IEnergyAssessmentGateway
{
    public async Task<EnergyAssessment> GetOrCreateAsync(
        EnergyAssessmentRequest request,
        CancellationToken cancellationToken)
    {
        var started = time.GetTimestamp();
        try
        {
            var result = await inner.GetOrCreateAsync(request, cancellationToken);
            metrics.RecordDependency("energy-partner", "success",
                time.GetElapsedTime(started));
            return result;
        }
        catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
        {
            metrics.RecordDependency("energy-partner", "caller-cancelled",
                time.GetElapsedTime(started));
            throw;
        }
        catch
        {
            metrics.RecordDependency("energy-partner", "failure",
                time.GetElapsedTime(started));
            throw;
        }
    }
}

Notice the cancellation filter. An internally imposed timeout may also surface as an OperationCanceledException; treating every cancellation as the caller abandoning the request produces misleading metrics. Use distinct cancellation sources or exception mapping so operations can distinguish them.

Decorator order is behaviour:

Authorisation -> Validation -> Idempotency -> Timeout -> Retry -> Adapter

This is only an example. Authorisation should happen before expensive work. Validation should happen before retries. An idempotency scope should cover the logical operation. Timeout outside retry caps the whole execution; timeout inside retry caps each attempt and can make the total much longer. Write the intended budget and test it with a fake clock or controlled server.

Never retry every exception. Invalid input, authentication failure and business conflict do not become valid with repetition. Retry transient network failures only where the operation is safe under the same key. Add jitter to reduce synchronised retry storms, and cap attempts within the caller’s deadline.

Make idempotency a persisted protocol

An Idempotency-Key header is not protection by itself. The server must bind it to actor, operation and request fingerprint, then persist the result.

public sealed record IdempotencyRecord(
    string TenantId,
    string Operation,
    string Key,
    string RequestHash,
    IdempotencyState State,
    string? ResponseJson,
    DateTimeOffset ExpiresAt);

When a command arrives:

1. Canonicalise the business request and calculate a cryptographic hash.
2. Attempt to create a record under a unique (tenant, operation, key) constraint.
3. If an existing completed record has the same hash, return its stored outcome.
4. If the key exists with a different hash, return a conflict.
5. If it is still processing, return a defined in-progress response or wait briefly.
6. Commit domain change, result and operation state according to one consistency design.

Junior: Can I keep processed keys in an in-memory dictionary?

>

Senior: That protects one process until restart. Production retries may reach another instance tomorrow. Persist keys for the retry window and enforce uniqueness in the database.

Do not store an ASP.NET response object. Store a stable business outcome that can be projected into the current API contract. Retention balances retry guarantees and storage/privacy requirements. The client contract should state the supported key format and window.

If the external provider accepts our key, propagate a derived key scoped to that dependency rather than leaking a general client secret. The same logical approval should reuse it across safe retries.

Concurrency: idempotency and optimistic versioning solve different races

Idempotency handles repeated intent. Optimistic concurrency handles competing intent. Two different approvers can send different idempotency keys against version seven. Only one should transition the aggregate.

public sealed class LoanApplicationConfiguration : IEntityTypeConfiguration<LoanApplication>
{
    public void Configure(EntityTypeBuilder<LoanApplication> builder)
    {
        builder.HasKey(x => x.Id);
        builder.Property(x => x.Version).IsConcurrencyToken();
        builder.OwnsOne(x => x.RequestedAmount);
    }
}

The command carries ExpectedVersion. The aggregate checks it for a meaningful domain error, and the persistence layer still enforces concurrency in case state changes between load and commit. Translate DbUpdateConcurrencyException into HTTP 409 with a safe instruction to refresh; do not blindly retry the whole approval using stale business assumptions.

Pessimistic locking may be appropriate for a short, highly contended database operation, but never hold a database transaction open across an HTTP call. The partner can take seconds or fail; locks would reduce throughput and risk deadlock. Use an explicit intermediate state or operation record, call outside the local transaction, then reload and compare version before applying the result.

That creates a business decision: if the application changes while an energy assessment is in flight, can the assessment still be used? Perhaps property identity is unchanged, or perhaps every material edit invalidates it. Put that rule in the domain and audit the evidence version.

Use the outbox for reliable events

Saving a decision and then publishing an event has a gap:

database commit succeeds
process crashes before broker publish
downstream systems never learn about approval

Publishing first has the opposite gap. The outbox stores the domain change and an event envelope in the same database transaction. A background dispatcher publishes pending envelopes and marks them delivered.

public sealed record OutboxMessage(
    Guid Id,
    string Type,
    string AggregateId,
    long AggregateVersion,
    string Payload,
    DateTimeOffset OccurredAt,
    DateTimeOffset? PublishedAt,
    int AttemptCount);

Delivery is normally at least once. A crash after broker acceptance but before PublishedAt can publish again. Consumers therefore use the message ID or a business idempotency key to deduplicate and design side effects safely.

Ordering needs an explicit guarantee. If several events for one application may publish concurrently, include aggregate version and let consumers reject, buffer or reconcile gaps. A global ordering guarantee is often expensive and unnecessary.

Do not put domain objects directly on the wire. Map to a versioned integration contract. Internal refactoring should not silently change consumers. Avoid sensitive fields unless the subscriber genuinely needs them and is authorised to receive them.

The dispatcher needs metrics for oldest pending age, pending count, attempts, dead-letter count and publication duration. A green approval API with a six-hour outbox backlog is not healthy. Provide a controlled replay operation that preserves message IDs and audit history.

Translate failures at the API boundary

The endpoint should not contain a chain of catch blocks. Central exception handling maps stable application failures into Problem Details:

public sealed class ApiExceptionHandler(ILogger<ApiExceptionHandler> logger)
    : IExceptionHandler
{
    public async ValueTask<bool> TryHandleAsync(
        HttpContext context,
        Exception exception,
        CancellationToken cancellationToken)
    {
        var (status, title, code) = exception switch
        {
            ValidationException => (400, "Request is invalid", "validation_failed"),
            ApplicationNotFoundException => (404, "Application not found", "not_found"),
            ConcurrencyConflictException => (409, "Application changed", "conflict"),
            PartnerUnavailableException => (503, "Assessment unavailable", "dependency_unavailable"),
            _ => (500, "Unexpected server error", "unexpected_error")
        };

        if (status == 500)
            logger.LogError(exception, "Unhandled failure {TraceId}", context.TraceIdentifier);

        await Results.Problem(
            statusCode: status,
            title: title,
            extensions: new Dictionary<string, object?>
            {
                ["code"] = code,
                ["traceId"] = context.TraceIdentifier
            }).ExecuteAsync(context);
        return true;
    }
}

The client sees no stack trace, SQL text or partner payload. Operations retain the exception and correlation ID under access-controlled logs. Expected 404 or validation responses should not flood error alerts; security-relevant patterns can still be monitored separately.

Map deliberately. Turning every exception into 400 blames the client for server failures and encourages unsafe retry behaviour. A 503 may tell a well-designed client that a request can be retried under the same idempotency key. Document Retry-After only when the server can provide meaningful guidance.

Debugging clinic: approvals occasionally appear twice

Imagine reporting shows duplicate LoanApproved notifications, although the database contains one approved application. We should not immediately blame the message broker.

Start with identifiers:

HTTP trace ID
command idempotency key
application ID and version
outbox message ID
broker message ID
consumer processing record
notification provider key

Trace one duplicate pair. If both notifications have the same outbox message ID, duplicate delivery is expected and the consumer’s deduplication failed. If they have different outbox IDs for the same aggregate version, event creation or transaction retry is wrong. If the consumer processed once but the email provider sent twice, the final side effect lacks idempotency or has an uncertain-timeout problem.

Junior: Could we switch to exactly-once delivery?

>

Senior: A marketing label will not make a database, broker and email provider one atomic transaction. Define where duplicates can occur and make each observable side effect idempotent or reconcilable.

The fix might be a unique consumer-inbox constraint on message ID, committed with the consumer’s local state change. For email, pass a stable provider idempotency key if supported. Otherwise persist a notification operation before sending and reconcile ambiguous results. Never mark delivery complete before the side effect unless the resulting loss is acceptable.

Add a regression test that delivers the same envelope twice and concurrently. Assert one business effect. Then kill the consumer between the external call and local acknowledgement and verify the recovery policy.

Security review of the same design

Defensive design begins at identity. The API derives tenant and approver from validated claims. It never trusts ApprovedBy or TenantId in JSON. The repository filters by tenant, and the domain operation checks the actor’s role and any separation-of-duties rule.

Object-level authorisation matters. Possessing the general LoanApprover role may not permit access to every branch, product or application. The authorizer should receive the resource or its security attributes, not only a role name.

Input limits belong at transport and domain boundaries. Limit string length, collection count and request size before expensive parsing or provider calls. Normalise only where semantics allow it; silently trimming an identifier may turn invalid input into a different valid entity.

Outbound HTTP needs an allow-listed base address and typed routes. Do not accept a callback URL from the request and fetch it from a privileged network, which creates server-side request forgery risk. Validate partner certificates through the platform defaults or approved configuration; never disable certificate validation to fix a development issue.

Secrets come from a managed secret store or workload identity, not source control. Rotate them and ensure logs redact headers. Encrypt sensitive data at rest and in transit, minimise what is persisted, define retention, and audit privileged access.

Threat-model business abuse too. An authorised operator might repeatedly request paid assessments, search sequential application IDs or approve their own exceptional case. Rate limits, resource authorisation, cost thresholds, non-enumerable identifiers and dual control may be appropriate depending on risk.

Test the architecture in layers

The domain suite tests calculations, transitions and invariants without infrastructure. Table-driven cases make policy boundaries visible:

[Theory]
[InlineData(4.74, "A", AssessmentOutcome.Approved)]
[InlineData(4.76, "A", AssessmentOutcome.ManualReview)]
[InlineData(4.24, "C", AssessmentOutcome.Approved)]
[InlineData(4.26, "C", AssessmentOutcome.ManualReview)]
public void Green_strategy_applies_rating_limit(
    decimal ratio, string rating, AssessmentOutcome expected)
{
    var context = AffordabilityFixture.WithRatioAndRating(ratio, rating);
    new GreenHomeAffordabilityStrategy().Assess(context).Outcome
        .Should().Be(expected);
}

Use-case tests prove orchestration: a completed idempotency record bypasses the partner; missing evidence refers; a version conflict performs no write. Fake the gateway with behaviour, not brittle expectations about every log call.

Adapter tests run against a controlled HTTP server. Verify headers, serialisation, unknown statuses, large responses, malformed JSON, timeout and cancellation. Record representative provider contracts with sensitive values removed, but do not let snapshots replace assertions about meaning.

Database integration tests use the production database engine in an isolated environment. Prove unique idempotency constraints, optimistic concurrency, outbox atomicity and tenant filtering. Two concurrent tasks should race in a test so only one expected transition commits.

API tests start the real application host with authentication fixtures. Verify 400, 401, 403, 404, 409, 503 and success contracts; confirm sensitive fields never appear. A 404-versus-403 policy must be intentional where resource existence is sensitive.

Resilience tests inject ambiguous timeouts. Property-based tests can explore monetary boundaries and sequences of domain commands. Load tests confirm connection pools and retry policies do not amplify a partner outage. None of these requires testing private methods.

Deployment and operational readiness

New code and schema may run alongside the old version during a rolling deployment. Database migrations must be backward compatible: add nullable structures or defaults first, deploy code that understands both shapes, backfill safely, then enforce or remove in a later release. A single destructive migration can make application rollback impossible.

Feature flags can expose the green product to internal users first, but flags are temporary operational state. Record owner and expiry. Both paths need testing until the old one is removed; leaving permanent dormant branches doubles risk.

Dashboards should connect technical and business signals:

• assessment requests, outcomes and duration by product;
• partner success, timeout, conflict and unknown-status rates;
• idempotency replays and mismatched-key conflicts;
• concurrency conflicts;
• outbox age, attempts and dead letters;
• API status and latency;
• manual-review reasons;
• paid searches per completed assessment.

Alerts need runbooks. A rise in manual review may be a legitimate product shift, a missing evidence feed or a mapping defect. An unknown partner status should page the owning integration team before it silently affects decisions. Logs should carry trace, application, safe tenant, operation and provider request identifiers without raw financial data.

For release, start with synthetic contract checks, then a test-tenant assessment, then limited product availability. Preserve the previous strategy and adapter configuration for rollback. If the database change is compatible and events are versioned, rollback is mechanical. If not, the team must document roll-forward recovery before deployment.

Junior: When is this feature done?

>

Senior: When the business path works, dangerous retries and races are controlled, support can diagnose it, rollback is credible, and the next developer can change the calculation without learning the partner protocol.

Exercises and review prompts

Exercise one: remove a premature pattern

Find an interface in a codebase with one trivial implementation. Explain the real axis of variation it protects. If none exists, inline it and compare test readability. The lesson is not that single-implementation interfaces are always wrong; boundaries for infrastructure or testing may be valuable. The exercise asks you to defend the cost.

Exercise two: model an ambiguous timeout

Write a fake partner that persists a request and then closes the connection before responding. Retry with the same idempotency key and prove only one paid assessment exists. Retry with the same key and different payload and prove the operation conflicts.

Exercise three: force a concurrency race

Load version five of the same application into two database contexts. Approve it in one and refer it in the other. Commit concurrently. Verify one succeeds and the other returns a meaningful conflict rather than overwriting state.

Exercise four: test event duplication

Deliver the same outbox envelope to a consumer twice, first sequentially and then concurrently. Persist message receipt with the business side effect. Confirm only one notification instruction is created.

Exercise five: write a pattern decision record

Choose Strategy, Decorator or Adapter from the example. State the pressure, simpler option, consequence and removal condition. Ask another developer to challenge it without using the pattern name. If the rationale survives, the pattern is probably serving the problem.

Cross-links for the next mentoring session

This guide focuses on code and boundary design. Continue with the site’s Pragmatic TDD in C# and .NET guide to practise driving these behaviours through tests. Use High-Performance C# and .NET when profiling allocation or concurrency rather than guessing. The EF Core Best Practices guide develops query, transaction and persistence concerns. The Microservices .NET guide explores when the outbox and integration contracts cross service boundaries. The Web Security guide expands identity, authorisation and trust-boundary thinking.

The order matters: make behaviour correct and understandable, prove it with appropriate tests, measure performance, and distribute it only when operational ownership demands distribution. Patterns support that journey; they are not a shortcut around it.

Refactoring clinic: rescue an over-patterned service

Patterns can create the problem they were meant to solve. Imagine the approval path now has an endpoint that sends a mediator command, a command handler that calls a coordinator, a coordinator that selects a workflow factory, a workflow that calls a domain service, and a generic repository that wraps EF Core. Every layer forwards the same identifiers. Finding the SQL query requires opening seven files.

Junior: The dependencies all point inward, so is this Clean Architecture?

>

Senior: Direction is useful, but navigation cost and cohesion still matter. If the layers add no policy, translation or isolation, they are indirection rather than architecture.

I would make a change map. For the last five approval features, list every file edited and why. If each change passes through the same forwarding types, mark them as candidates for collapse. Then use tests at stable public boundaries to protect behaviour while simplifying.

Suppose the handler contains only this:

public Task<AssessmentResult> Handle(
    AssessGreenLoanCommand command,
    CancellationToken cancellationToken) =>
    coordinator.AssessAsync(command, cancellationToken);

This may be required by the chosen messaging library, but it does not deserve independent business documentation or unit tests. Keep it as a thin adapter and put the readable use case behind it—or invoke the use case directly from the endpoint if in-process dispatch adds no needed pipeline behaviour.

A generic repository with GetAll, Find, Add, Update and Delete often leaks persistence while hiding EF Core’s useful query model. Replace it with a focused port only where the domain needs one:

public interface ILoanApplicationRepository
{
    Task<LoanApplication?> LoadAsync(
        LoanApplicationId id,
        TenantId tenant,
        CancellationToken cancellationToken);

    Task<bool> ExistsWithExternalReferenceAsync(
        TenantId tenant,
        string reference,
        CancellationToken cancellationToken);
}

Read-only reporting may query a projection through a dedicated query service instead of forcing every query through an aggregate repository. Command and query needs differ; clarity matters more than symmetry.

Remove one layer at a time. Run domain, integration and API tests after each step. Compare generated SQL and telemetry because a simplification can accidentally introduce N+1 queries or lose a span. Keep commits small enough to review. The goal is not the fewest classes; it is the shortest accurate path from business idea to implementation.

Review clinic: challenge a pull request constructively

Assume the junior developer submits the green-loan change. A weak review says, “Too complex—simplify,” or “Use Polly,” without identifying a risk. A mentoring review anchors every comment in behaviour.

For example:

The partner POST is retried after any exception with a newly generated key.
If the first request commits and its response is lost, a retry can create a
second paid assessment. Please create the operation key before the retry scope,
reuse it on every attempt, and add an ambiguous-timeout integration test.

This comment states evidence, consequence, requested direction and proof. It leaves room for a better solution. Severity should reflect impact: possible duplicate financial side effect is blocking; a private method name is usually not.

The author should respond with reasoning rather than silently applying every suggestion. Perhaps the partner contract guarantees a natural unique property reference. If so, capture the contract and test the duplicate response. Review is collaborative risk analysis, not a seniority contest.

I would review in passes:

1. Read the requirement and architecture decision without looking at syntax.
2. Trace the successful business path.
3. Trace invalid input, dependency failure, cancellation, retry and concurrency.
4. Check identity, authorisation, sensitive data and outbound trust.
5. Inspect tests against the claimed risks.
6. Consider deployment compatibility, observability and rollback.
7. Finally review naming, duplication and local readability.

This ordering prevents twenty style comments from hiding a missing tenant filter. Automated formatting and analyzers should handle mechanical consistency before human review.

Junior: How do I know when to approve with a small concern?

>

Senior: Ask whether the concern can cause incorrect behaviour, security exposure, irreversible operational pain or an expensive near-term change. If not, label it non-blocking and let the author choose. Reviews need trust as well as rigour.

Failure-mode table for defensive design

A compact table makes assumptions reviewable:

The table is not a substitute for code. It is a review index linking each risk to an enforcement point, observable signal and test. If a row says “log error and continue,” challenge whether correctness is being sacrificed invisibly.

Some failures require product decisions. If the energy provider is down, should every application be referred, queued for later assessment or rejected temporarily? Engineering can explain consistency, cost and user experience, but a domain owner must choose. Encode the choice as policy and test it; do not let an exception handler accidentally decide lending behaviour.

Definition of done for this worked feature

I would not call the green-loan change complete until the following evidence exists:

• Product calculations and effective policy version are approved and tested at boundaries.
• Domain construction and transitions reject impossible state.
• Partner contract tests cover success, unknown values, duplicates and ambiguous timeout.
• Idempotency persists across instances and rejects key reuse with different input.
• Concurrency tests prove stale commands cannot overwrite newer decisions.
• Decision and outbox message commit atomically.
• Consumers tolerate duplicate delivery.
• API authentication, resource authorisation and safe Problem Details are tested.
• Sensitive data is absent from routine logs and unauthorised responses.
• Dashboards, alerts and runbooks cover the defined failure table.
• Database and event changes are compatible with rolling deployment and rollback.
• The architecture decision records why each important pattern exists and its cost.
• A developer outside the feature team can trace the path and operate a synthetic failure.

This definition is longer than “all unit tests pass” because production correctness spans time and systems. It is also bounded: it does not demand a pattern, mock or document for every class. Every artefact answers a known risk or maintenance need.

There is one final mentoring habit I would add: revisit the design after the feature has lived in production. Compare our predictions with evidence. Did product rules really vary independently? Did the adapter contain partner changes? Were retries observed, and did idempotency work? Which alerts helped, and which produced noise? How many files did the next product change touch?

If Strategy never gained a second meaningful behaviour and only increases navigation, simplify it. If provider changes repeatedly leak beyond the adapter, strengthen that boundary. If decorators make latency budgets impossible to understand, consolidate or expose their effective policy. If the outbox is reliable but painful to support, improve tooling rather than returning to unsafe direct publication.

Architecture decisions are hypotheses about future change and failure. Production supplies the evidence. Schedule a short review after the first significant change or incident, update the decision record, remove obsolete flags and tests, and keep the code aligned with what the system has become. Defensive design is not maximal abstraction on day one; it is the ability to change safely when our first assumptions prove incomplete.

That feedback loop is the difference between collecting patterns and practising engineering. The team learns which boundaries absorb change, which controls prevent incidents and which abstractions merely consume attention. Those lessons should shape the next design review before a familiar pattern is proposed again.

What I want you to take away

Clean code makes intent visible. SOLID principles help us reason about responsibility, extension and dependency direction. Design patterns give names to recurring structures. Defensive programming prepares those structures for reality.

Use Strategy when behaviour genuinely varies. Use Factory when construction decisions deserve one home. Use Decorator for composable cross-cutting behaviour, while testing order and security. Use Facade and Adapter to shield the domain from external complexity. Use Command when an intention needs auditability, validation or independent handling. Use events for genuinely decoupled reactions, with idempotency and versioned contracts. Use Builder when staged construction or readable test data justifies it.

Then look beyond the class diagram. Validate every trust boundary. Preserve domain invariants. Treat nullability warnings seriously. Design safe retries. Pass cancellation. Control concurrency. Keep transactions local. Protect personal data. Document API contracts. Test the database and network behaviours that mocks cannot prove. Review failure modes before production teaches them to you.

The mature question is never, “How many patterns did we use?” It is, “Can another developer understand this, can the system survive change, and have we made the dangerous failures difficult?”

That is the standard I want you to carry into your next C# design review.