Preventing Fragile C# Changes from Reaching Main with Code Metrics, Analyzers, and Safe Resource Handling

A pull request can be green, pass a quick manual test, and still introduce code that is difficult to maintain or unsafe to operate. The compiler confirms that the code is syntactically valid. It does not automatically prove that every exception is visible, every file handle is released, every decision path is understandable, or every team member follows the same coding rules.

Consider a .NET service that creates settlement export files for an external partner. A developer adds support for several export channels. The new method converts user input, chooses a channel, formats a payload, writes a file, and catches every possible exception. It appears to work with valid test data, so the team is tempted to approve it.

The real risk appears later. Invalid input is silently converted into a misleading result. A file remains locked because its stream was never disposed. Adding one more export channel requires another nested condition. Reviewers disagree about whether the method is acceptable because the team has no measurable or automated quality standard.

The goal is not to make every method mathematically perfect. The goal is to create a repeatable path that turns fragile code into readable code and prevents the same problems from returning through future pull requests.

The Problem in One Method

The first implementation mixes validation, routing, file access, output formatting, and error handling:

public sealed class SettlementExportService
{
    public int Export(string itemCountText, string channelCode, byte[] payload)
    {
        try
        {
            int itemCount = Convert.ToInt32(itemCountText);
            FileStream output = new("settlement.dat", FileMode.Create);

            if (channelCode == "P")
            {
                if (itemCount > 0)
                {
                    output.Write(payload, 0, payload.Length);
                    Console.WriteLine("Partner export completed");
                }
            }
            else if (channelCode == "A")
            {
                if (itemCount > 0)
                {
                    output.Write(payload, 0, payload.Length);
                    Console.WriteLine("Archive export completed");
                }
            }
            else if (channelCode == "R")
            {
                if (itemCount > 0)
                {
                    output.Write(payload, 0, payload.Length);
                    Console.WriteLine("Review export completed");
                }
            }

            return itemCount;
        }
        catch
        {
            return 0;
        }
    }
}

This code has several independent failure modes:

• Convert.ToInt32 uses an exception for a predictable validation failure.
• The empty catch hides formatting errors, file system errors, and programming defects.
• The FileStream is created but never deterministically disposed.
• Repeated branches increase the number of paths through the method.
• Magic strings such as "P", "A", and "R" do not explain their intent.
• One method owns too many responsibilities.
• An unsupported channel produces no explicit result.

A successful execution with one valid input does not cancel these risks. The method is fragile because its behavior becomes harder to reason about as new branches and operational failures are added.

Use Metrics as Review Signals, Not as a Scoreboard

Code metrics help a team replace vague comments such as "this feels complicated" with observable signals. They should guide investigation and refactoring, not become a game where developers optimize numbers without improving the design.

In Visual Studio, calculate metrics for the complete solution rather than only the currently selected project. Start with the methods that combine low maintainability with high complexity. These are often better refactoring targets than large but straightforward data structures.

Metrics also need context. A generated file can be large without being a design problem. A small method can still be dangerous if it swallows exceptions or leaks resources. Use several signals together and inspect the actual code before deciding what to change.

Refactor the Decision Paths Before Adding More Features

The export method becomes easier to understand when raw strings are translated into an enum and each responsibility moves into a focused method.

public enum ExportChannel
{
    Partner,
    Archive,
    Review
}

public sealed class SettlementExportService
{
    public int Export(string itemCountText, string channelCode, byte[] payload)
    {
        if (!int.TryParse(itemCountText, out int itemCount) || itemCount <= 0)
        {
            return 0;
        }

        ExportChannel channel = ParseChannel(channelCode);
        WritePayload("settlement.dat", payload);
        WriteCompletionMessage(channel);

        return itemCount;
    }

    private static ExportChannel ParseChannel(string channelCode) =>
        channelCode switch
        {
            "P" => ExportChannel.Partner,
            "A" => ExportChannel.Archive,
            "R" => ExportChannel.Review,
            _ => throw new ArgumentOutOfRangeException(
                nameof(channelCode),
                channelCode,
                "Unsupported export channel")
        };

    private static void WriteCompletionMessage(ExportChannel channel)
    {
        string message = channel switch
        {
            ExportChannel.Partner => "Partner export completed",
            ExportChannel.Archive => "Archive export completed",
            ExportChannel.Review => "Review export completed",
            _ => throw new ArgumentOutOfRangeException(nameof(channel), channel, null)
        };

        Console.WriteLine(message);
    }

    private static void WritePayload(string path, byte[] payload)
    {
        using FileStream output = new(path, FileMode.Create);
        output.Write(payload, 0, payload.Length);
    }
}

This refactoring improves the design in several ways:

1. TryParse treats malformed numeric input as an expected validation outcome instead of an exceptional event.
2. The switch expressions make supported values visible in one place.
3. The fallback branch handles unexpected values explicitly.
4. File writing has one responsibility and one resource lifetime.
5. Adding a new channel no longer requires copying the file-writing logic.
6. Method names communicate intent without explanatory comments.

The important change is not the use of a particular syntax feature. The improvement comes from separating decisions, validation, and side effects so each part can be reviewed independently.

If channel-specific processing becomes more substantial, move each implementation behind an interface instead of extending the switch indefinitely. Composition usually keeps dependencies clearer than a deep inheritance tree. The service can depend on a small contract while concrete handlers own their specialized behavior.

Handle Exceptions at a Meaningful Boundary

The first version catches every exception and returns zero. That creates a dangerous ambiguity. A caller cannot distinguish an invalid item count from a disk failure, an unauthorized path, or an unexpected programming defect.

A safer approach separates expected input failures from operational exceptions. Validate predictable conditions without exceptions. Catch only exceptions that the current layer can log, translate, compensate for, or otherwise handle meaningfully.

using Microsoft.Extensions.Logging;

public sealed class SettlementExportRunner
{
    private readonly ILogger<SettlementExportRunner> _logger;
    private readonly SettlementExportService _service;

    public SettlementExportRunner(
        ILogger<SettlementExportRunner> logger,
        SettlementExportService service)
    {
        _logger = logger;
        _service = service;
    }

    public int Run(string itemCountText, string channelCode, byte[] payload)
    {
        try
        {
            return _service.Export(itemCountText, channelCode, payload);
        }
        catch (IOException exception)
        {
            _logger.LogError(
                exception,
                "The settlement export file could not be written for channel {ChannelCode}",
                channelCode);

            throw;
        }
        catch (ArgumentOutOfRangeException exception)
        {
            _logger.LogWarning(
                exception,
                "The export request contained an unsupported channel {ChannelCode}",
                channelCode);

            throw;
        }
    }
}

This boundary does not pretend that every failure is recoverable. It records useful context and preserves the failure by rethrowing it. Another application boundary can translate the exception into an API response, a failed job state, or an operational alert.

Avoid surrounding every small method with try-catch. Excessive local handling can make control flow harder to follow and can accidentally hide defects. Prefer a small number of deliberate boundaries where the application has enough context to make a useful decision.

Also avoid exposing stack traces or raw exception details to end users. Internal diagnostics and user-facing error messages serve different purposes.

Make Resource Ownership Explicit

The garbage collector manages the lifetime of managed objects, but it does not remove the need to release operating system resources promptly. File handles, sockets, streams, and similar resources commonly implement IDisposable because their lifetime should be deterministic.

Use this ownership rule:

• When a method creates a disposable object for short-term use, dispose it with using.
• When a class creates and retains a disposable object, the class owns it and should implement IDisposable.
• When a dependency is injected, the component that created it normally owns its disposal. Do not dispose an injected shared service unless its ownership contract explicitly says otherwise.

A sealed class that owns a long-lived stream can use a simple disposal implementation:

public sealed class ExportAuditWriter : IDisposable
{
    private readonly FileStream _stream;
    private bool _disposed;

    public ExportAuditWriter(string path)
    {
        _stream = new FileStream(path, FileMode.Append, FileAccess.Write);
    }

    public void Append(byte[] entry)
    {
        if (_disposed)
        {
            throw new ObjectDisposedException(nameof(ExportAuditWriter));
        }
        _stream.Write(entry, 0, entry.Length);
    }

    public void Dispose()
    {
        if (_disposed)
        {
            return;
        }

        _stream.Dispose();
        _disposed = true;
    }
}

Because the class is sealed and owns only managed disposable resources, it does not need an extensible disposal hierarchy. More complex cases that own unmanaged resources require the full disposal pattern, including cleanup that remains safe when disposal is not called directly.

The key review question is simple: who created the resource, and where is its lifetime closed? If the answer is unclear, the code has an ownership problem even if it currently appears to work.

Move Coding Standards into the Repository

A coding standard that exists only in a document or one developer's IDE settings cannot reliably protect the codebase. The standard should travel with the repository and be checked by the build.

A practical enforcement stack has several layers:

Developer change
    |
    v
IDE diagnostics and code cleanup
    |
    v
Repository code-style configuration
    |
    v
Project analyzers
    |
    v
Build with warnings treated as errors
    |
    v
Pull request review and code metrics
    |
    v
Merge to main

Use each layer for a different purpose:

• IDE diagnostics provide immediate feedback while code is being written.
• Code Cleanup removes avoidable formatting and unused-code noise before a commit.
• .editorconfig stores shared style decisions with the codebase.
• Built-in .NET analyzers detect correctness, reliability, and quality problems.
• Third-party analyzers can add stricter or specialized rules.
• Build enforcement ensures that local IDE differences do not bypass the standard.
• Pull request review evaluates design decisions that a static rule cannot fully understand.
• Code metrics help reviewers identify methods and classes that deserve deeper inspection.

At minimum, enable nullable reference analysis and fail the build when warnings violate the agreed quality bar:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>net10.0</TargetFramework>
    <Nullable>enable</Nullable>
    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
  </PropertyGroup>
</Project>

Treating warnings as errors should be introduced deliberately. First establish a clean baseline, agree on severities, and avoid enabling a large rule set that nobody has time to fix. A quality gate that developers routinely suppress is weaker than a smaller set of rules the team consistently respects.

Prefer analyzers distributed with the project over tools installed only as IDE extensions. Repository-level dependencies are versioned, repeatable, and available to every developer and build agent. IDE extensions can still improve the local experience, but they should not be the only enforcement mechanism.

Define the Pull Request Workflow

The branching model should match the release process, but every model needs a controlled review path. For teams that keep main continuously releasable, use short-lived branches and require a pull request before merge. For controlled release cycles, apply the same checks to both ongoing development and release branches, with stricter promotion rules where needed.

A useful pull request sequence is:

1. Create a focused branch for one change.
2. Refactor high-risk code before adding another branch or responsibility.
3. Run code cleanup so formatting noise does not hide behavioral changes.
4. Build with analyzers and warnings-as-errors enabled.
5. Calculate metrics for the affected project or solution.
6. Review exception boundaries and resource ownership explicitly.
7. Verify that unsupported inputs have defined behavior.
8. Merge only after automated checks and human review agree.

A reviewer should not approve merely because the output looks correct. The reviewer should also ask whether the implementation is safe to copy as a future team pattern.

Verify the Refactoring with Failure-Oriented Tests

Do not test only the successful export. The quality problems in this scenario are visible mainly in failure paths and repeated execution.

Verify at least these cases:

• A valid count and supported channel write the expected payload.
• A malformed count is rejected without throwing a parsing exception.
• A zero or negative count does not create an export.
• An unsupported channel produces an explicit failure.
• A file system error is logged and remains observable to the caller.
• The output file can be reopened, moved, or deleted after the operation, proving that the stream was released.
• Repeated calls do not accumulate locked handles.
• The build fails when a configured analyzer reports an error-level violation.
• The refactored method stays within the team's complexity target.

This test list connects implementation details to operational outcomes. It verifies not only what the code returns, but also whether failures remain diagnosable and resources remain available.

Common Mistakes

Catching Exception everywhere

A broad catch can be appropriate at a top-level boundary, but it is usually harmful inside ordinary business logic. Catch specific exceptions when the method can make a meaningful decision. Otherwise, allow the failure to reach the layer responsible for handling it.

Returning a normal value after an abnormal failure

Returning 0, false, or an empty string after an unrelated operational error hides the difference between valid and failed execution. Use an explicit result, a well-defined exception, or a boundary-level translation.

Assuming the garbage collector closes resources immediately

Garbage collection is not a substitute for deterministic disposal. Use using, try-finally, or a correct IDisposable implementation when the code owns disposable resources.

Treating one metric as an absolute rule

A complexity score is a signal, not a complete design review. Combine metrics with knowledge of the domain, failure behavior, coupling, and change frequency.

Configuring standards only on developer machines

Local settings create inconsistent results. Store style rules and analyzers with the repository, then enforce the chosen severities during the build.

Refactoring only to reduce numbers

Moving every branch into a tiny method can reduce a metric while making navigation worse. Refactor around responsibilities and meaningful concepts, not only around thresholds.

Pull Request Quality Checklist

• [ ] Expected validation failures do not rely on exceptions.
• [ ] Empty catch blocks are absent.
• [ ] Caught exceptions are specific and handled meaningfully.
• [ ] Errors contain enough diagnostic context for operators.
• [ ] Disposable resources have an explicit owner and lifetime.
• [ ] Switches and switch expressions handle unexpected values.
• [ ] Magic strings and numbers are replaced with named concepts where useful.
• [ ] Methods have one clear responsibility.
• [ ] Complexity, coupling, inheritance depth, and size were reviewed together.
• [ ] Shared style rules are versioned in the repository.
• [ ] Analyzers run during the build, not only inside one IDE.
• [ ] Required warnings fail the build.
• [ ] Failure paths and repeated execution are tested.
• [ ] The code is safe for another developer to copy as a team pattern.

Conclusion

Reliable C# code is not defined only by whether it compiles or succeeds with ideal input. It must also expose failures, release owned resources, keep decision paths understandable, and remain safe to change.

The strongest improvement comes from combining several practices. Refactor complicated methods around clear responsibilities. Use exceptions for exceptional conditions rather than routine validation. Dispose resources according to ownership. Measure code to locate risk. Store coding rules with the repository. Enforce those rules in the build and review the remaining design decisions in pull requests.

That workflow turns code quality from a subjective cleanup activity into a normal part of delivery, making it much harder for fragile patterns to reach main unnoticed.