Imported from TarasKovalenko/clean-code-dotnet-skills (SKILL.md). Install upstream with npx skills add TarasKovalenko/clean-code-dotnet-skills. Copyright stays with the author.

Clean Code for .NET

This skill applies the ideas of Clean Code (Robert C. Martin) to C# and .NET. The book's examples are Java; this skill translates the principles into idiomatic modern .NET (C# 10+, .NET 8+) and flags where the .NET ecosystem has a better-fitting answer.

The one idea behind everything here: code is read far more often than it is written, so optimize for the next reader. Every rule below is a means to that end. When a rule would make the code harder to read in a specific case, the reader wins and the rule loses. Say so explicitly when you bend a rule.

Pick the mode

Figure out which of these the user wants, then follow that section.

1. Write new code → follow the Core Rules while writing; don't lecture, just produce clean code and briefly note notable design choices.
2. Review existing code → use the Review workflow and output format below.
3. Refactor existing code → use the Refactor workflow below.
4. Explain / teach a principle → explain in your own words with a small before/after C# example.

Reference files (read only what you need)

For a full code review, always read references/smells-catalog.md since it gives the IDs used in the review output.

Bundled tools

• scripts/scan_smells.py <file-or-dir> [--json] [--max-method-lines N] [--max-params N] [--only-async]: regex-based first pass over .cs files (skips bin/, obj/, generated files). Findings are hints tagged with catalog IDs; always confirm by reading the code. --only-async limits output to the A* async/concurrency checks.
• assets/.editorconfig: naming conventions and analyzer severities aligned with this skill (including async analyzers CA2012/CA2016/CA1849/CA2008 and VSTHRD rules). Offer it when the user asks about enforcing style, or when a review finds many formatting/naming issues.
• assets/Directory.Build.props: nullable, analyzers, code-style-in-build, warnings-as-errors in CI. Offer alongside the .editorconfig.

Core Rules (the short version)

Names

• Names reveal intent: elapsedTimeInDays, not d. If a name needs a comment, rename it.
• Follow .NET conventions: PascalCase types/methods/properties, camelCase locals/parameters, _camelCase private fields, I prefix for interfaces, Async suffix for awaitable methods.
• Classes are nouns (InvoiceCalculator); methods are verbs (CalculateTotal). Avoid vague nouns like Manager, Helper, Processor, Data, Info unless nothing more precise exists.
• No Hungarian notation, no type encodings (strName, lstOrders), no noise words (OrderObject, TheCustomer).
• One word per concept across the codebase (don't mix Get/Fetch/Retrieve for the same idea).

Functions (methods)

• Small, and doing one thing at one level of abstraction. A method that has sections ("// validate", "// save", "// notify") is doing several things: extract them.
• Rough guide: most methods under ~20 lines; review anything over ~40. Readability decides, not a line counter.
• Few parameters: 0–2 ideal, 3 needs a reason, 4+ → introduce a parameter object or record.
• No boolean flag parameters (Render(true)) → split into two well-named methods.
• No hidden side effects. A method named CheckPassword must not also start a session.
• Command-query separation: a method either changes state or returns information, not both (well-known exceptions like TryParse and Dictionary.TryAdd are fine).
• Prefer guard clauses and early returns over deep nesting.
• Don't repeat yourself; extract duplicated logic, but don't merge code that only looks alike and changes for different reasons.

Comments

• Good code explains itself; a comment is often a sign a name or extraction is missing.
• Keep: why comments, legal headers, warnings of consequences, TODO with an owner/ticket, and XML doc comments (///) on public APIs of libraries.
• Remove: commented-out code (source control remembers), comments restating the code, journal/changelog comments, closing-brace comments, and #region blocks used to hide a class that is too big.

Formatting

• Enforce with tooling (.editorconfig + dotnet format + analyzers), not with review comments. See assets/.editorconfig.
• Newspaper order: public API first, then the private helpers it calls, roughly in call order.
• One public type per file; file name matches type name; file-scoped namespaces.

Classes and SOLID

• Classes are small in responsibilities, not just lines. If you can't describe the class in one sentence without "and", split it.
• Depend on abstractions injected through the constructor (Microsoft.Extensions.DependencyInjection); never new up services inside business logic.
• Prefer composition over inheritance. Keep interfaces small and client-specific.
• High cohesion: most methods should use most fields.

Objects vs data structures

• Objects hide data and expose behavior; data structures (DTOs, records) expose data and have no behavior. Don't build hybrids.
• Law of Demeter: avoid order.Customer.Address.City.Name train wrecks in business logic; ask the object to do the work.
• Use record / record struct for immutable value-like data; use init and required for construction safety.

Error handling

• Use exceptions for exceptional situations, not for control flow. Consider a Result type for expected business failures (validation, "not found") if the codebase already uses one.
• Don't return null for collections (return empty); enable nullable reference types (<Nullable>enable</Nullable>) and respect the annotations.
• Don't pass null into methods; guard with ArgumentNullException.ThrowIfNull(x).
• Rethrow with throw; (never throw ex;). Don't catch Exception except at application boundaries (middleware, top-level handlers). Never swallow exceptions silently.
• Exceptions carry context: specific type + message saying what failed and with which input.

Boundaries

• Wrap third-party APIs behind your own interface so the rest of the code isn't coupled to them. Use IHttpClientFactory, typed clients, and the Options pattern.
• Write learning tests when adopting a new library.

Unit tests

• Tests are production-quality code: readable, one concept per test, Arrange-Act-Assert.
• F.I.R.S.T.: Fast, Independent, Repeatable, Self-validating, Timely.
• Name tests by behavior: MethodName_Scenario_ExpectedResult or a sentence-style name; be consistent with the project.

Async and concurrency

Async bugs rarely fail loudly; they starve the thread pool, deadlock UIs, leak memory, or lose exceptions. Treat these as correctness rules, not style (details in references/async-await.md):

• Async all the way. Never block on tasks: no .Result, .Wait(), .GetAwaiter().GetResult(), Task.WaitAll, including in constructors, Dispose, timer callbacks, and GetOrAdd factories.
• No async void except event handlers, and those wrap their body in try/catch. Never pass an async lambda where an Action is expected (List.ForEach, Parallel.ForEach, callback APIs); use foreach + await, Parallel.ForEachAsync, or Func<Task> overloads.
• Every task has an owner. Await it, Task.WhenAll it, or queue it to a BackgroundService via Channel<T>. In UI apps, use async commands or a safe fire-and-forget helper that routes exceptions to a handler.
• Cancellation flows. Accept CancellationToken as the last parameter and pass it to everything; dispose timeout/linked CancellationTokenSources; use Task.WaitAsync for operations that don't take a token.
• Prefer await over returning the inner task, and never elide inside using/try. Use Task.FromResult/Task.CompletedTask/ValueTask for precomputed results, not Task.Run.
• Prefer await over ContinueWith; create TaskCompletionSource with RunContinuationsAsynchronously.
• Dispose asynchronously (await using) for writers and IAsyncDisposable resources.
• ConfigureAwait(false) in libraries; not needed in ASP.NET Core app code; not before UI access.
• Avoid AsyncLocal<T>; prefer explicit state or DI.
• Keep concurrency mechanics separate from business logic; prefer immutable data and Channel<T> over manual locking.

The Boy Scout Rule

Leave the code a little cleaner than you found it: when touching a file, fix one small thing nearby (a name, a dead using, an extracted method), without turning a bug fix into a rewrite.

Pragmatism (important)

Clean Code is a set of heuristics, not law, and some advice predates modern C#. Apply judgment:

• Don't shred readable 25-line methods into ten 3-line ones that force the reader to jump around. Extraction must add a meaningful name.
• Don't add interfaces for classes with one implementation and no test seam need, just to satisfy DIP ("interface for everything" is its own smell).
• LINQ pipelines, pattern matching, switch expressions, and records are clean C# idioms; prefer them over Java-style verbose equivalents.
• Respect the existing codebase's conventions over this skill's defaults when they conflict, and mention the conflict instead of silently rewriting.
• Performance-critical code (hot paths, Span<T>, pooled buffers) may justifiably trade some readability; require a comment explaining why. The same applies to eliding async/await or using ValueTask: fine when measured and safe, not by habit.
• Async rules are about correctness, so don't relax them for brevity. When you must block (a sync interface you can't change), do it once at a documented boundary and say why.

Review workflow

1. Read the whole snippet/file first to understand intent before judging.
2. Read references/smells-catalog.md (and any topic files relevant to what you see). If the code contains async, Task, ValueTask, timers, background work, or UI commands, also read references/async-await.md; async defects usually rank High because they cause outages rather than readability problems.
3. If the code is in files and Python is available, you can run python scripts/scan_smells.py <path> for a quick mechanical pass (long methods, many parameters, magic numbers, commented-out code, throw ex;, empty catches, and async pitfalls such as sync-over-async, async void, async lambdas passed as Action, ContinueWith, TaskCompletionSource options, undisposed CancellationTokenSources, LongRunning with async delegates, elided awaits inside using, and AsyncLocal). Treat its output as hints to verify, not as the review.
4. Rank findings by impact: correctness/bug risk → design/maintainability → readability → style.
5. Skip pure-formatting nitpicks if an .editorconfig/formatter would fix them; mention the tooling once instead.

Review output format

Use this structure:

## Summary
<2–4 sentences: overall quality, the biggest issue, what's already good>

## Findings
### 1. <Short title> — <Severity: High | Medium | Low> — <Smell ID(s), e.g. F1, E2>
**Where:** `ClassName.MethodName` (line ~N)
**Why it matters:** <one or two sentences in terms of readability/change risk>
**Suggestion:**
<small C# before/after, or a description if the change is large>

(repeat, most important first; usually 3–10 findings)

## Refactored version (optional)
<full cleaned-up code when the snippet is small enough and the user wants it>

## Next steps
<prioritized, e.g. "add tests around X before extracting Y">

Mention genuine strengths in the summary. A review that only criticizes is less useful and less credible.

Refactor workflow

1. Safety net first. Check whether tests exist. If not, suggest (or write) characterization tests before changing behavior-bearing code.
2. Small, behavior-preserving steps, each one nameable: Rename, Extract Method, Introduce Parameter Object, Replace Conditional with Polymorphism, Extract Class, Inline Temp, Replace Magic Number with Constant, Introduce Guard Clause.
3. Keep behavior identical unless the user asked for changes. If you notice a bug, call it out separately rather than silently fixing it inside a refactor.
4. Show the result and list the steps taken with the reason for each, so the user can apply them incrementally (and as separate commits if they like).
5. Keep public API signatures stable unless the user agrees to break them.

Quick example

Before:

public bool Process(Order o, bool send)
{
    if (o != null)
    {
        if (o.Items.Count > 0)
        {
            decimal t = 0;
            foreach (var i in o.Items) t += i.Price * i.Qty;
            if (t > 1000) t = t * 0.95m; // discount
            o.Total = t;
            _db.Orders.Add(o);
            _db.SaveChanges();
            if (send) _mail.Send(o.Email, "Order confirmed", "Total: " + t);
            return true;
        }
    }
    return false;
}

After:

private const decimal BulkDiscountThreshold = 1000m;
private const decimal BulkDiscountRate = 0.05m;

public async Task PlaceOrderAsync(Order order, CancellationToken cancellationToken)
{
    ArgumentNullException.ThrowIfNull(order);
    if (order.Items.Count == 0)
        throw new InvalidOperationException($"Order {order.Id} has no items.");

    order.Total = CalculateTotal(order.Items);
    await _orders.AddAsync(order, cancellationToken);
}

public async Task SendConfirmationAsync(Order order, CancellationToken cancellationToken) =>
    await _notifications.SendOrderConfirmationAsync(order, cancellationToken);

private static decimal CalculateTotal(IEnumerable<OrderItem> items)
{
    var subtotal = items.Sum(item => item.Price * item.Quantity);
    return subtotal > BulkDiscountThreshold
        ? subtotal * (1 - BulkDiscountRate)
        : subtotal;
}

What changed: vague Process → intent-revealing names; flag argument removed (caller decides whether to send); nesting replaced with guard clauses; magic numbers named; pricing logic extracted; persistence and email hidden behind abstractions; async with cancellation. Note the behavior change (exceptions instead of false) and confirm it with the user in a real refactor.