dotnet-csharp-code-smells

Proactive code-smell and anti-pattern detection for C# code. This skill triggers during all workflow modes -- planning, implementation, and review. Each entry identifies the smell, explains why it is harmful, provides the correct fix, and references the relevant CA rule or cross-reference.

Scope

• Resource management (IDisposable misuse)

• Async anti-patterns and deadlock detection

• DI lifetime misuse and captive dependencies

• Null-handling mistakes and NRT violations

• LINQ pitfalls and string handling issues

Out of scope

• LLM-specific generation mistakes (wrong NuGet packages, MSBuild errors) -- see [skill:dotnet-agent-gotchas]

• SOLID/DRY design principles -- see [skill:dotnet-solid-principles]

• Naming and style conventions -- see [skill:dotnet-csharp-coding-standards]

Cross-references: [skill:dotnet-csharp-async-patterns] for async gotchas, [skill:dotnet-csharp-coding-standards] for naming and style, [skill:dotnet-csharp-dependency-injection] for DI lifetime misuse, [skill:dotnet-csharp-nullable-reference-types] for NRT annotation mistakes.

1. Resource Management (IDisposable Misuse)

Smell Why Harmful Fix Rule

Missing using on disposable locals Leaks unmanaged handles (sockets, files, DB connections) Wrap in using declaration or using block CA2000

Undisposed IDisposable fields Class holds disposable resource but never disposes it Implement IDisposable ; dispose fields in Dispose()

CA2213

Wrong Dispose pattern (no finalizer guard) Double-dispose or missed cleanup on GC finalization Follow canonical Dispose(bool) pattern; call GC.SuppressFinalize(this)

CA1816

Disposable created in one method, stored in field Ownership unclear; easy to forget disposal Document ownership; make the containing class IDisposable

CA2000

using on non-owned resource Premature disposal of shared resource (e.g., injected HttpClient ) Only dispose resources you create; let DI manage injected services

See details.md for code examples of each pattern.

2. Warning Suppression Hacks

Smell Why Harmful Fix Rule

Invoking event with null to suppress CS0067 Creates misleading runtime behavior; masks real bugs Use #pragma warning disable CS0067 or explicit event accessors { add {} remove {} }

CS0067

Dummy variable assignments to suppress CS0219 Dead code that confuses readers Use _ = expression; discard or #pragma warning disable

CS0219

Blanket #pragma warning disable without restore Suppresses ALL warnings for rest of file Always pair with #pragma warning restore ; suppress specific codes only

[SuppressMessage] without justification Future maintainers cannot evaluate if suppression is still valid Always include Justification = "reason"

CA1303

See details.md for the CS0067 motivating example (bad pattern to correct fix).

3. LINQ Anti-Patterns

Smell Why Harmful Fix Rule

Premature .ToList() mid-chain Forces full materialization; wastes memory Keep chain lazy; materialize only at the end CA1851

Multiple enumeration of IEnumerable<T>

Re-executes query or DB call on each enumeration Materialize once with .ToList() then reuse CA1851

Client-side evaluation in EF Core Loads entire table into memory; silent perf bomb Rewrite query as translatable LINQ or use AsAsyncEnumerable() with explicit intent

.Count() > 0 instead of .Any()

Enumerates entire collection instead of short-circuiting Use .Any() for existence checks CA1827

Nested foreach instead of .Join() or .GroupJoin()

O(n*m) when O(n+m) is possible Use LINQ join operations or Dictionary lookup

.Where().First() instead of .First(predicate)

Creates unnecessary intermediate iterator Pass predicate directly to .First() or .FirstOrDefault()

CA1826

4. Event Handling Leaks

Smell Why Harmful Fix Rule

Not unsubscribing from events Memory leak: publisher holds reference to subscriber Unsubscribe in Dispose() or use weak event pattern

Raising events in constructor Subscribers may not be attached yet; derived class not fully constructed Raise events only from fully initialized instances CA2214

async void event handler (misused) async void is the only valid signature for event handlers, but exceptions are unobservable Wrap body in try/catch; log and handle exceptions explicitly

Event handler not checking for null NullReferenceException when no subscribers Use event?.Invoke() null-conditional pattern

Static event without cleanup Rooted references prevent GC for application lifetime Prefer instance events or use WeakEventManager

--

Cross-reference: [skill:dotnet-csharp-async-patterns] covers async void fire-and-forget patterns in depth.

5. Design Smells

Smell Threshold Why Harmful Fix

God class

500 lines Too many responsibilities; hard to test and maintain Extract cohesive classes using SRP

Long method

30 lines Hard to understand, test, and review Extract helper methods with descriptive names

Long parameter list

5 parameters Indicates missing abstraction Introduce parameter object or builder

Feature envy Method uses another class's data more than its own Misplaced responsibility; tight coupling Move method to the class it envies

Primitive obsession Domain concepts represented as raw string /int

No type safety; validation scattered Introduce value objects or strongly-typed IDs

Deep nesting

3 levels of indentation Hard to follow control flow Use guard clauses (early return) and extract methods

6. Exception Handling Gaps

Smell Why Harmful Fix Rule

Empty catch block Silently swallows errors; masks bugs At minimum, log the exception; prefer letting it propagate CA1031

Catching base Exception

Catches OutOfMemoryException , StackOverflowException , etc. Catch specific exception types CA1031

Log-and-swallow (catch { log; } ) Caller never learns operation failed Re-throw after logging, or return error result

Throwing in finally

Masks original exception with the new one Use try/catch inside finally; never throw from finally

throw ex; instead of throw;

Resets stack trace; hides original failure location Use bare throw; to preserve stack trace CA2200

Not including inner exception Loses causal chain when wrapping exceptions Pass original as innerException parameter

Cross-reference: [skill:dotnet-csharp-async-patterns] covers exception handling in fire-and-forget and async void scenarios.

Quick Reference: CA Rules

Rule Description

CA1031 Do not catch general exception types

CA1816 Call GC.SuppressFinalize correctly

CA1826 Do not use Enumerable methods on indexable collections

CA1827 Do not use Count() /LongCount() when Any() can be used

CA1851 Possible multiple enumerations of IEnumerable collection

CA2000 Dispose objects before losing scope

CA2200 Rethrow to preserve stack details

CA2213 Disposable fields should be disposed

CA2214 Do not call overridable methods in constructors

Enable these via <AnalysisLevel>latest-all</AnalysisLevel> in your project. See [skill:dotnet-csharp-coding-standards] for analyzer configuration.

References

• Microsoft Code Quality Rules

• Framework Design Guidelines

• David Fowler Async Guidance