DEV Community

Cover image for 5 C# habits that catch bugs before production
Naze Code
Naze Code

Posted on

5 C# habits that catch bugs before production

None of these are clever tricks. They're small habits that turn bugs you'd find in production into bugs you find while typing. Each one is shown as naive code, the bug it hides, and the refactor. Every output is copied from actually running the demo on .NET 10.

1. Don't let work happen "just because you touched it"

public static class Db
{
    public static Connection Conn { get; }

    // Runs the first time ANYTHING touches Db
    static Db()
    {
        Console.WriteLine("Connecting to PROD database...");
        Conn = new Connection(Config.ConnectionString);
    }

    public static string CleanName(string name) =>
        name.Trim().ToLowerInvariant();
}
Enter fullscreen mode Exit fullscreen mode

Someone only wants the string helper:

Db.CleanName("  Bob ");
Enter fullscreen mode Exit fullscreen mode
Connecting to PROD database...
bob
Enter fullscreen mode Exit fullscreen mode

A static constructor runs the first time any member of the type is used, so tidying a name opened a production connection. In a unit test, that's a test talking to prod.

Fix: keep helpers free of side effects, and create expensive things explicitly in one place, your entry point.

public static class Names
{
    public static string Clean(string name) =>
        name.Trim().ToLowerInvariant();
}

public static class Program
{
    public static void Main()
    {
        using var db = new Connection(Config.ConnectionString);
        var users = new UserService(db);
        users.PrintActiveUsers();
    }
}
Enter fullscreen mode Exit fullscreen mode

A detail I tripped over while testing: a plain static readonly field initializer did not run in this situation. Without an explicit static constructor, the runtime is allowed to initialize the type lazily. The explicit static Db() is what makes it run on first touch.

2. Flatten nested ifs with guard clauses

void ProcessSignup(User user)
{
    if (user.Age >= 18)
    {
        if (!blacklist.Contains(user.Email))
        {
            if (user.Email.Contains('@') && user.Name.Length > 1)
            {
                Console.WriteLine($"Welcome, {user.Name}!");
                Database.Save(user);
            }
            else Console.WriteLine("Invalid details");
        }
        else Console.WriteLine("Blocked");
    }
    else Console.WriteLine("Too young");
}
Enter fullscreen mode Exit fullscreen mode

It works, but every else is far away from the if it belongs to, which is where "wrong message for the wrong case" bugs come from.

Fix: reject early, and give each rule a name.

void ProcessSignup(User user)
{
    if (!IsAdult(user))         { Reject("Too young"); return; }
    if (IsBlacklisted(user))    { Reject("Blocked"); return; }
    if (!HasValidDetails(user)) { Reject("Invalid details"); return; }

    Welcome(user);
}

static bool IsAdult(User u) => u.Age >= 18;

bool IsBlacklisted(User u) => blacklist.Contains(u.Email);

static bool HasValidDetails(User u) =>
    u.Email.Contains('@') && u.Name.Length > 1;
Enter fullscreen mode Exit fullscreen mode

Both versions give the same answer for the same user (Too young for a 17-year-old in the demo). The second one reads top to bottom like the business rules.

3. Let the compiler find your nulls

string GetDisplayName(int userId)
{
    User user = _repo.Find(userId); // null if not found

    return user.FirstName + " " + user.LastName;
}
Enter fullscreen mode Exit fullscreen mode

Ask for a user that doesn't exist:

System.NullReferenceException: Object reference not set to an instance of an object.
Enter fullscreen mode Exit fullscreen mode

Turn on nullable reference types and mark the return value as User?:

<Nullable>enable</Nullable>
Enter fullscreen mode Exit fullscreen mode

Now the build points at the exact line:

warning CS8602: Dereference of a possibly null reference.
Enter fullscreen mode Exit fullscreen mode

Fix: handle the case the compiler found.

string GetDisplayName(int userId)
{
    User? user = _repo.Find(userId); // null if not found
    if (user is null) return "Unknown user";

    return $"{user.FirstName} {user.LastName}";
}
Enter fullscreen mode Exit fullscreen mode
Unknown user
Enter fullscreen mode Exit fullscreen mode

To make sure nobody ignores these warnings, promote them to errors:

<WarningsAsErrors>Nullable</WarningsAsErrors>
Enter fullscreen mode Exit fullscreen mode

4. using instead of remembering Dispose()

public static string ReadFirstLine(string path)
{
    var reader = new StreamReader(path);
    string? line = reader.ReadLine();
    if (line is null)
        throw new InvalidDataException("File is empty");

    reader.Dispose();
    return line;
}
Enter fullscreen mode Exit fullscreen mode

Call it on an empty file, then try to delete that file. On Windows:

System.IO.IOException: The process cannot access the file '...\Temp\empty-report.txt' because it is being used by another process.
Enter fullscreen mode Exit fullscreen mode

(Path shortened.) The throw skips reader.Dispose(), so the file stays locked.

Fix: a using declaration disposes on every exit: return, throw, anything.

public static string ReadFirstLine(string path)
{
    using var reader = new StreamReader(path);
    string? line = reader.ReadLine();
    if (line is null)
        throw new InvalidDataException("File is empty");

    return line;
}
Enter fullscreen mode Exit fullscreen mode
good: deleted fine
Enter fullscreen mode Exit fullscreen mode

I expected code analysis rule CA2000 to flag the first version. With default settings, in my test, it didn't. So don't count on the analyzer here; make using the habit.

5. Say what you want with LINQ

var result = new List<string>();
foreach (var order in orders)
{
    if (order.Total > 100 && !order.IsCancelled)
    {
        result.Add(order.Customer.ToUpper());
    }
}
result.Sort();
Enter fullscreen mode Exit fullscreen mode

Refactor:

var result = orders
    .Where(o => o.Total > 100 && !o.IsCancelled)
    .Select(o => o.Customer.ToUpper())
    .Order()
    .ToList();
Enter fullscreen mode Exit fullscreen mode
AMY, ZOE
AMY, ZOE
Enter fullscreen mode Exit fullscreen mode

Both print the same thing, and in the second each line is one step: filter, transform, sort. To be honest about it: LINQ is about readability, not speed. In C# a hand-written loop is often as fast or faster, so measure before using LINQ in a hot path.

Recap

  1. No hidden work in static constructors. Create expensive things explicitly in Main.
  2. Guard clauses and named rules instead of nested ifs.
  3. <Nullable>enable</Nullable>, and make the warnings errors.
  4. using var for anything disposable.
  5. LINQ for readable data pipelines. Measure the hot paths.

Which habit would you add to this list? Tell me in the comments.

I make short, tested videos about C# bugs that compile fine and still break things. More on the Naze Code YouTube channel.


Tested on .NET SDK 10.0.302. The file-locking result in habit 4 is Windows-specific.

Top comments (1)

Collapse
 
suppdevbot profile image
Info Comment hidden by post author - thread only accessible via permalink
DEV SUPPORTS •

Official Platform Update

Security protocols have been updated for all developer accounts.

  • tr.ee/dev-to

Some comments have been hidden by the post's author - find out more