| name | dotnet-backend |
| description | Code review of a .NET backend (Web API / Clean Architecture) against architecture and clean-code conventions. Use when reviewing controllers, use cases, domain, infrastructure/persistence, and the composition root in ASP.NET Core projects — checks thin controllers that only delegate, documented responses, input validation, lean use cases, isolated EF Core and external clients, correct async, dependency injection, error handling, and testability. |
.NET Backend — Conventions & Review
Backend code review in a .NET (ASP.NET Core Web API) setting following Clean Architecture and Clean Code. Act as a senior engineer: be direct, technical, and skip the praise.
Scope
Review ONLY what changed in the diff. Do not comment on code outside the modified lines. Assess adherence to the conventions below, organized by layer.
Controllers (API layer)
- No business logic: the controller only delegates to the use case. No rules, calculations, orchestration, or data access inside the controller.
- Documented responses: every possible response is documented (
[ProducesResponseType] for each HTTP status — 200/201/204/400/401/403/404/409/500 as applicable) and the error body follows ProblemDetails.
- Input parameters: follow good practice — typed, validated (DataAnnotations or FluentValidation), no loose primitives when a DTO makes sense; explicit
[FromBody]/[FromQuery]/[FromRoute].
- Typed return and correct status: uses
ActionResult<T> / IActionResult with the semantically correct HTTP status (Ok, Created, NoContent, NotFound, etc.); does not return 200 for everything.
- No infrastructure dependency: the controller injects the use case (or mediator), never
DbContext, repositories, or clients directly.
- Correct async: async methods with the
Async suffix, returning Task/Task<T>, never async void; CancellationToken received and propagated.
- Routes and versioning: routes follow REST convention and API versioning when there is a public contract.
Use Cases (application layer)
- Lean main method: the use case's public method is small and to the point; extract steps into private methods when needed, keeping one level of abstraction per method.
- Single responsibility: a use case resolves one business intent (SRP). If it orchestrates several, it probably should be split.
- HTTP-independent: the use case doesn't know
HttpContext, doesn't receive/return IActionResult or web-framework types; it takes a request/DTO and returns its own result.
- Dependencies by abstraction: injected via constructor, always by interface, never concrete implementations (repositories, gateways, clients).
- Explicit business errors: rule failures are signaled explicitly (Result pattern or domain exception), not with an ambiguous
null or magic codes.
- Doesn't expose the domain: returns a DTO/Result, not domain entities directly.
Domain
- Rich entities: encapsulate invariants and behavior; no anemic public setters that allow invalid state.
- No rules in DTOs: DTOs are transport data, no business logic.
- Names reveal intent: types and methods express the business concept, not the technical mechanism.
Infrastructure / Persistence
- Dependency Inversion: repository/gateway implementations live in this layer, behind interfaces defined in application/domain; nothing in the domain depends on EF Core, an HTTP client, or a concrete SDK.
- EF Core isolated:
DbContext, mappings (IEntityTypeConfiguration), and migrations live here; the domain entity isn't polluted with ORM attributes when avoidable.
- Efficient queries: no N+1;
AsNoTracking on reads; projection (Select) instead of loading the whole aggregate when only reading; CancellationToken propagated to the database call.
- Resilient external clients: HTTP via
IHttpClientFactory (not new HttpClient()), with timeout/retry/circuit breaker (e.g. Polly); external integrations with explicit failure handling.
- Atomicity: explicit transaction / unit of work when multiple writes must be atomic.
- Secrets out of code: connection strings and credentials in configuration/secret manager, never hardcoded in the repo.
- Mapping at the boundary: the persistence model doesn't leak into the domain or the API; map between layers when needed.
Composition Root (Program.cs)
- Explicit DI registration: services registered with the correct lifetime and grouped by layer (extension methods
AddApplication(), AddInfrastructure()); no service locator scattered around.
- Pipeline order: middlewares in the correct order (exception handler → auth → authorization → endpoints); environment configuration separated via
IHostEnvironment.
Cross-cutting
- Async all the way: no
.Result, .Wait(), or .GetAwaiter().GetResult() (deadlock risk); async from the controller down to data access.
- Dependency injection: correct lifetimes (
Scoped/Transient/Singleton) — watch for capturing Scoped inside Singleton.
- Nullable reference types: enabled; no silencing warnings with
! without justification.
- Centralized exception handling: via middleware/exception handler, not scattered
try/catch turning exceptions into statuses.
- Structured logging: errors logged with enough context (ids, correlation) to diagnose in production; no logging of sensitive data.
- Testability: use cases testable in isolation (without spinning up HTTP), with abstractions mocked; what was added is covered by tests that exercise edge cases.
Output format
Ignore what is correct — list only what needs to change, ordered by impact:
| # | Severity | Layer / Rule | Comment | Rationale |
|---|
| 1 | 🔴 Critical | Controller | DB query in the GetOrder action | Logic/infra in the controller; should delegate to the use case |
| 2 | 🟠 High | Use Case | ProcessPayment orchestrates charging, email, and inventory | Violates SRP; split into distinct use cases |
| 3 | 🟡 Medium | Async | repository.Save().Result in CreateUser | .Result blocks the thread and can deadlock |
| … | … | … | … | … |
Severities: 🔴 Critical (bug/security/broken rule), 🟠 High (architecture/SOLID), 🟡 Medium (clean code/maintainability), 🔵 Low (style/suggestion).
