C# Code Review Checklist: .NET Best Practices, Tools, and Examples

A good C# code review catches problems that compile just fine but come back to cost you later.

null handling based on luck. async code that blocks under load. Entity Framework queries that work with ten rows and fall apart with ten thousand. IDisposable objects that live longer than they should. These are the kinds of problems a C# reviewer needs to know how to spot.

This checklist is for reviewing real C# and .NET pull requests. Use it before approving changes to ASP.NET Core APIs, workers, libraries, desktop apps, or any service where correctness matters.

Quick C# code review checklist

Use this first pass when you need to review quickly.

• Does the code do what the PR says it does?
• Are nullable reference types enabled and handled correctly?
• Are async methods awaited with await, without .Result or .Wait()?
• Is cancellation passed through to long-running asynchronous operations?
• Are IDisposable and IAsyncDisposable resources disposed of correctly?
• Do Entity Framework queries make it explicit when related data is loaded?
• Are LINQ queries materialized only when needed?
• Are dependencies injected instead of created inside business logic?
• Are exceptions specific, useful, and safe to log?
• Are inputs validated before reaching the database, filesystem, network, or shell?
• Do the tests cover the changed behavior?
• Are style issues handled by analyzers instead of manual review comments?

Check correctness before style

Start with behavior.

Read the PR description, the ticket, and the changed code together. Then verify that the implementation actually delivers what was proposed. An authorization fix should affect the real access path to the endpoint. A change meant to prevent duplicate processing needs some way to guarantee idempotency. And a performance optimization should target a known bottleneck or, at minimum, a plausible hot path.

Questions to ask:

• Does the code match the expected behavior?
• Are edge cases covered, such as empty collections, missing records, invalid IDs, timeouts, and retries?
• Could this change break an existing caller?
• Does the method name still describe what it does?
• Did the change introduce any hidden side effects, such as extra database writes or background work?

A useful comment is specific:

// Risk: this updates the invoice before the payment provider confirms the charge.
invoice.Status = InvoiceStatus.Paid;
await paymentProvider.ChargeAsync(invoice.Total);

Better feedback:

This marks the invoice as paid before the external charge finishes. Can we move the status update to after ChargeAsync, or first store a separate PaymentPending state?

Review null handling

C# gives the compiler tools to catch null problems, but only if the project uses them properly.

Check the .csproj:

<PropertyGroup>
  <Nullable>enable</Nullable>
</PropertyGroup>

Microsoft describes nullable reference types as a compile-time feature that helps catch potential NullReferenceException issues before runtime. Newer .NET templates enable them by default, but older projects may still have them turned off. See Microsoft’s guide to nullable reference types.

Look for these problems:

• ! used to silence warnings without a real guarantee.
• Public methods returning null without a nullable return type.
• DTOs where required fields can be missing.
• FirstOrDefault() followed by direct property access.
• Dictionary lookups using the indexer when the key may not exist.

Problematic code:

var customer = customers.FirstOrDefault(c => c.Id == customerId);
return customer.Email.ToLowerInvariant();

Better:

var customer = customers.FirstOrDefault(c => c.Id == customerId);

if (customer is null)
{
    throw new CustomerNotFoundException(customerId);
}

return customer.Email.ToLowerInvariant();

If Email can be null, the type should say so with string?, and the caller should handle it.

Review async code and cancellation

Async code in C# can fail in pretty unremarkable ways. That is exactly why it deserves attention during review.

Microsoft’s task-based asynchronous pattern recommends Task and Task<TResult> for asynchronous operations, a predictable Async suffix, and CancellationToken parameters when an operation supports cancellation. See the .NET task-based asynchronous pattern.

Look for:

• .Result or .Wait() inside code that could be async.
• async void outside event handlers.
• Missing await.
• Fire-and-forget tasks with no ownership or logging.
• A CancellationToken accepted by a method but dropped in downstream calls.
• Sequential awaits for independent operations that could use Task.WhenAll.

Problematic code:

public UserProfile GetProfile(Guid userId)
{
    var user = _users.GetByIdAsync(userId).Result;
    var orders = _orders.GetRecentAsync(userId).Result;

    return new UserProfile(user, orders);
}

Better:

public async Task<UserProfile> GetProfileAsync(
    Guid userId,
    CancellationToken cancellationToken)
{
    var userTask = _users.GetByIdAsync(userId, cancellationToken);
    var ordersTask = _orders.GetRecentAsync(userId, cancellationToken);

    await Task.WhenAll(userTask, ordersTask);

    return new UserProfile(await userTask, await ordersTask);
}

In library code, be intentional about context capture. In application code, consistency usually matters more than adding ConfigureAwait(false) everywhere without a team-wide rule.

Review resource disposal

Anything that implements IDisposable needs a clear lifetime. Files, streams, timers, database connections, and many framework objects should not depend on the garbage collector to release external resources.

Microsoft’s C# reference explains that a using statement disposes an IDisposable instance when control leaves the block, even if an exception occurs. It also covers await using for IAsyncDisposable. See the documentation for the C# using statement.

Problematic code:

var stream = File.OpenRead(path);
return await JsonSerializer.DeserializeAsync<Order>(stream);

Better:

await using var stream = File.OpenRead(path);
return await JsonSerializer.DeserializeAsync<Order>(stream, cancellationToken);

Be careful with HttpClient. The review question is not just “is it being disposed?”. In many .NET applications, HttpClient should be created through IHttpClientFactory so connection pooling works correctly.

Review Entity Framework and LINQ

A C# PR can look clean and still hide a query problem.

Look for loops that trigger database calls, navigation properties loaded unintentionally, and queries materialized too early with ToList() or ToArray(). The EF Core documentation covers eager loading with Include and ThenInclude for related data. See eager loading in EF Core.

Problematic code:

var orders = await _db.Orders.ToListAsync(cancellationToken);

foreach (var order in orders)
{
    Console.WriteLine(order.Customer.Name);
}

Better:

var orders = await _db.Orders
    .Include(order => order.Customer)
    .Where(order => order.Status == OrderStatus.Open)
    .ToListAsync(cancellationToken);

Also check whether the query returns more data than necessary. If the endpoint needs five fields, project five fields.

var orders = await _db.Orders
    .Where(order => order.Status == OrderStatus.Open)
    .Select(order => new OrderSummary(
        order.Id,
        order.Number,
        order.Customer.Name,
        order.Total))
    .ToListAsync(cancellationToken);

Good review comments are concrete here: “this can generate one query per order” is more useful than “performance issue.”

Review dependency injection and testability

In C# applications, dependency injection is often the difference between code that is easy to test and code you can only poke through an HTTP endpoint.

Look for dependencies created inside business logic:

public class InvoiceService
{
    public async Task SendInvoiceAsync(Invoice invoice)
    {
        var client = new SmtpClient();
        await client.SendMailAsync(BuildMessage(invoice));
    }
}

Better:

public class InvoiceService
{
    private readonly IEmailSender _emailSender;

    public InvoiceService(IEmailSender emailSender)
    {
        _emailSender = emailSender;
    }

    public Task SendInvoiceAsync(
        Invoice invoice,
        CancellationToken cancellationToken)
    {
        return _emailSender.SendAsync(BuildMessage(invoice), cancellationToken);
    }
}

Ask:

• Can this code be tested without a real network, database, clock, or filesystem?
• Are the dependencies registered with the correct lifetime?
• Is a scoped service being captured by a singleton?
• Has business logic ended up trapped inside controllers or handlers?

Review exceptions and logs

Exceptions should explain the failure without leaking secrets.

Microsoft’s guidance says exceptions should be thrown when a method cannot complete its defined function, and sensitive information should not be included in exception messages. See Microsoft’s guide to creating and throwing exceptions.

Look for:

• catch (Exception) blocks that swallow errors.
• Logs containing passwords, tokens, API keys, or full request bodies.
• throw new Exception() instead of a specific exception type.
• Losing the original stack trace with throw ex;.
• Returning null for error states that should be explicit.

Problematic code:

try
{
    await processor.ProcessAsync(command);
}
catch (Exception ex)
{
    _logger.LogError(ex.Message);
}

Better:

try
{
    await processor.ProcessAsync(command, cancellationToken);
}
catch (PaymentProviderException ex)
{
    _logger.LogError(
        ex,
        "Payment provider failed for order {OrderId}",
        command.OrderId);

    throw;
}

If the code can recover, recover explicitly. If it cannot, log useful context and let the error propagate to the caller or global handler.

Review security boundaries

A lot of security review in C# starts at input boundaries.

Check data coming from:

• HTTP request bodies and route parameters.
• Query strings.
• Headers.
• Webhooks.
• Files.
• Queues.
• Environment variables.
• Admin panels and internal tools.

Look for:

• SQL built through string concatenation.
• User input passed into file paths.
• Missing authorization checks on changed endpoints.
• Over-posting in ASP.NET Core model binding.
• Secrets written to logs.
• Deserializing untrusted payloads without restrictions.

Risky SQL:

var sql = $"SELECT * FROM Users WHERE Email = '{email}'";

Better: use parameterized queries or EF Core query APIs.

var user = await _db.Users
    .SingleOrDefaultAsync(user => user.Email == email, cancellationToken);

Security review also includes defaults. A new endpoint should make the required permission obvious in the code, not hide it behind a convention nobody remembers.

Review tests

Tests should prove the changed behavior, not just increase coverage.

Ask:

• Would the test fail without the production change?
• Does it cover the edge case that motivated the PR?
• Are async paths awaited in the tests?
• Are database tests isolated?
• Are time, random values, and external services controlled?
• Does the test name describe the behavior?

Bad example:

[Fact]
public async Task Test1()

Better to describe it like this:

[Fact]
public async Task CreateOrderAsync_returns_error_when_customer_is_missing()

For a bug fix, look for a regression test. For a refactor, look for existing tests that still describe the expected behavior.

Automate style with analyzers

Review time is too expensive to spend debating brace placement.

Use analyzers and .editorconfig for style and quality rules. Microsoft’s Roslyn analyzer documentation explains that analyzers inspect C# code for style, quality, maintainability, design, and related issues. Code style analyzers are included in the .NET SDK starting with .NET 5 and can be enforced as warnings or build errors. See Microsoft’s documentation on Roslyn analyzers and configuring rules with .editorconfig.

A useful baseline:

[*.cs]
dotnet_diagnostic.CA1822.severity = warning
dotnet_style_qualification_for_field = false:suggestion
dotnet_style_predefined_type_for_locals_parameters_members = true:suggestion

For C# teams, consider:

• .NET SDK analyzers.
• .editorconfig.
• StyleCop Analyzers.
• Roslynator.
• Sonar analyzers.
• xUnit analyzers for test projects.
• GitHub Actions or another CI step running dotnet build and dotnet test.

The rule is simple: if a machine can catch something reliably, a machine should catch it before a person opens the PR.

Code review tools for C#

A good C# code review setup combines analyzers, CI tests, and human review. The right tool depends on the kind of problem you want to catch: style, bugs, security issues, regressions, or business context.

Tool	Best use in C#/.NET projects	What it catches well	Where it fits in the workflow
Kodus	AI pull request review with team-specific rules	Logic issues, recurring patterns, missing tests, risks across multi-file changes, and project-specific rules	During the pull request
.NET analyzers	Native .NET SDK analysis	Quality warnings, style issues, and some common C# problems	Local build and CI
.editorconfig	Style standardization	Naming, formatting, language preferences, and rule severity	IDE, build, and CI
StyleCop Analyzers	Stricter C# style rules	Code conventions, XML documentation, ordering, and consistency	IDE and CI
Roslynator	C# refactoring and inspections	Simplifications, lightweight code smells, and idiomatic improvements	IDE and local review
SonarQube or SonarCloud	Project-level quality and security	Bugs, code smells, duplication, vulnerabilities, and security hotspots	CI and quality gates
ReSharper or Rider inspections	Fast feedback during development	Refactoring, nullability, LINQ, async, dead code, and C#-specific suggestions	IDE
xUnit analyzers	Test quality	Misuse of assertions, fixtures, async tests, and fragile test patterns	IDE and CI
GitHub Actions	Automated execution of checks	dotnet build, dotnet test, analyzers, coverage, and security checks	Before merge

A practical rule: use analyzers for what is deterministic, CI to stop obvious regressions, and Kodus to review what depends on PR context, repository patterns, and the intent behind the change.

What reviewers should block in a C# PR

Not every comment needs to block the merge. In C#, I would block a PR when the change introduces a real production risk.

Block when you see:

• .Result or .Wait() in an async flow that can cause deadlocks or tie up threads under load.
• async void outside event handlers.
• A CancellationToken ignored in a long-running operation, HTTP request, database query, or job.
• IDisposable or IAsyncDisposable with no clear disposal path.
• An EF Core query with an N+1 risk in an important endpoint or job.
• External input used in SQL, file paths, or shell calls without validation.
• catch (Exception) swallowing errors or hiding critical failures.
• A secret, token, or sensitive payload being written to logs.
• A change to a public contract without tests or updates to callers.
• New business logic without tests covering the main path and at least one failure case.

Non-blocking comments should be treated as improvements, not toll gates. Naming, small style preferences, and internal organization should only block when they violate a project rule or make the code harder to maintain.

Final checklist before approving

Before approving a C# pull request, check:

• The behavior matches the PR description.
• Nullable warnings were not ignored.
• Async code does not block.
• Cancellation is passed through where it matters.
• Disposable resources have a clear lifetime.
• Queries do not hide N+1 problems.
• Dependencies can be injected.
• Exceptions preserve useful context.
• Logs do not expose secrets.
• Tests cover the changed behavior.
• Style feedback is automated whenever possible.

A good C# code review is practical. It catches the problems that become expensive after the merge while they are still cheap to fix. That is the point.