C# Anti-Patterns

C# Anti-Patterns

Common C# Anti-Patterns with Fixes and xUnit Tests

Production-quality code samples and tests, one section per pattern.

1️⃣ NullReferenceException → Use ?. or explicit null checks

1
2
3
4
5
6
7
8

namespace AntiPatterns
{
    public static class NullSafety
    {
        public static int GetNameLength(string? name)
            => name?.Length ?? 0;
    }
}

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15

using Xunit;
using AntiPatterns;

public class NullSafetyTests
{
    [Theory]
    [InlineData("Mark", 4)]
    [InlineData("",     0)]
    [InlineData(null,   0)]
    public void GetNameLength_ReturnsExpected(string? input, int expected)
    {
        Assert.Equal(expected,
                     NullSafety.GetNameLength(input));
    }
}

2️⃣ String concatenation in loops → Switch to StringBuilder

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17

namespace AntiPatterns
{
    public static class ReportBuilder
    {
        public static string BuildReport(IEnumerable<int> values)
        {
            var sb = new StringBuilder(
                values is ICollection<int> c ? c.Count * 4 : 256);
            foreach (var v in values)
                sb.Append(v).Append(", ");

            return sb.Length > 2
                ? sb.ToString(0, sb.Length - 2)
                : string.Empty;
        }
    }
}

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21

using System;
using Xunit;
using AntiPatterns;

public class ReportBuilderTests
{
    [Fact]
    public void BuildReport_ReturnsCommaSeparatedList()
    {
        var list   = new[] { 1, 2, 3 };
        var result = ReportBuilder.BuildReport(list);
        Assert.Equal("1, 2, 3", result);
    }

    [Fact]
    public void BuildReport_EmptyCollection_ReturnsEmpty()
    {
        Assert.Equal(string.Empty,
                     ReportBuilder.BuildReport(Array.Empty<int>()));
    }
}

3️⃣ Deadlocks with async/await → Never use .Result—await instead

1
2
3
4
5
6
7
8
9
10
11
12
13
14

namespace AntiPatterns
{
    public interface IApiClient { Task<string> GetDataAsync(); }

    public class DataLoader
    {
        private readonly IApiClient _api;
        public DataLoader(IApiClient api) => _api = api;

        public async Task<string> LoadAsync()
            => await _api.GetDataAsync()
                         .ConfigureAwait(false);
    }
}

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19

using System.Threading.Tasks;
using Moq;
using Xunit;
using AntiPatterns;

public class DataLoaderTests
{
    [Fact]
    public async Task LoadAsync_ReturnsData()
    {
        var mock = new Mock<IApiClient>();
        mock.Setup(x => x.GetDataAsync()).ReturnsAsync("OK");

        var loader = new DataLoader(mock.Object);
        var data   = await loader.LoadAsync();

        Assert.Equal("OK", data);
    }
}

4️⃣ Unmanaged resource leaks → Always Dispose() or use using

1
2
3
4
5
6
7
8
9
10
11

namespace AntiPatterns
{
    public static class FileSaver
    {
        public static void Save(string path, byte[] data)
        {
            using var fs = new FileStream(path, FileMode.Create);
            fs.Write(data, 0, data.Length);
        }
    }
}

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25

using System;
using System.IO;
using Xunit;
using AntiPatterns;

public class FileSaverTests : IDisposable
{
    private readonly string _temp = Path.GetTempFileName();

    public void Dispose()
    {
        if (File.Exists(_temp))
            File.Delete(_temp);
    }

    [Fact]
    public void Save_WritesExpectedBytes()
    {
        var bytes = new byte[] { 1, 2, 3 };
        FileSaver.Save(_temp, bytes);

        var read = File.ReadAllBytes(_temp);
        Assert.Equal(bytes, read);
    }
}

5️⃣ Wrong string comparison → Use .Equals() with StringComparison

1
2
3
4
5
6
7
8
9
10
11

namespace AntiPatterns
{
    public static class StringComparerUtil
    {
        public static bool IsAdmin(string input)
            => string.Equals(
                    input,
                    "admin",
                    StringComparison.OrdinalIgnoreCase);
    }
}

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16

using Xunit;
using AntiPatterns;

public class StringComparerUtilTests
{
    [Theory]
    [InlineData("admin", true)]
    [InlineData("Admin", true)]
    [InlineData("ADMIN", true)]
    [InlineData("user",  false)]
    public void IsAdmin_IsCaseInsensitive(string input, bool expected)
    {
        Assert.Equal(expected,
                     StringComparerUtil.IsAdmin(input));
    }
}

6️⃣ Modifying collections while iterating → Use .RemoveAll() or snapshot

1
2
3
4
5
6
7
8
9
10

namespace AntiPatterns
{
    public class Session { public bool IsExpired { get; set; } }

    public static class SessionManager
    {
        public static void RemoveExpired(List<Session> sessions)
            => sessions.RemoveAll(s => s.IsExpired);
    }
}

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23

using System.Collections.Generic;
using System.Linq;
using Xunit;
using AntiPatterns;

public class SessionManagerTests
{
    [Fact]
    public void RemoveExpired_RemovesOnlyExpired()
    {
        var sessions = new List<Session>
        {
            new() { IsExpired = true  },
            new() { IsExpired = false },
            new() { IsExpired = true  }
        };

        SessionManager.RemoveExpired(sessions);

        Assert.Single(sessions);
        Assert.False(sessions.First().IsExpired);
    }
}

7️⃣ Swallowing exceptions → Log errors and rethrow strategically

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31

namespace AntiPatterns
{
    public class DataProcessor
    {
        private readonly ILogger<DataProcessor> _logger;
        private readonly Func<Task>              _work;

        public DataProcessor(
            ILogger<DataProcessor> logger,
            Func<Task>? work = null)
        {
            _logger = logger;
            _work   = work ?? DefaultWorkAsync;
        }

        public async Task ProcessAsync()
        {
            try
            {
                await _work().ConfigureAwait(false);
            }
            catch (Exception ex)
            {
                _logger.LogError(ex, "Error in ProcessAsync");
                throw;
            }
        }

        private Task DefaultWorkAsync() => Task.CompletedTask;
    }
}

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31

using System;
using System.Threading.Tasks;
using Moq;
using Xunit;
using Microsoft.Extensions.Logging;
using AntiPatterns;

public class DataProcessorTests
{
    [Fact]
    public async Task ProcessAsync_OnFailure_LogsAndRethrows()
    {
        var logger    = new Mock<ILogger<DataProcessor>>();
        Func<Task> f  = () => throw new InvalidOperationException("fail");
        var processor = new DataProcessor(logger.Object, f);

        var ex = await Assert.ThrowsAsync<InvalidOperationException>(
            () => processor.ProcessAsync());

        // Verify the underlying Log method is called with the correct parameters
        logger.Verify(
            x => x.Log(
                LogLevel.Error, // Expected log level
                It.IsAny<EventId>(), // EventId can be anything
                It.Is<It.IsAnyType>((v, t) => v.ToString().Contains("Error in ProcessAsync")), // Check if the message contains the expected text
                ex, // The expected exception
                It.IsAny<Func<It.IsAnyType, Exception?, string>>() // Formatter function can be anything
            ),
            Times.Once); // Ensure it was called exactly once
    }
}

8️⃣ Overusing var → Explicit types when unclear

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18

namespace AntiPatterns
{
    public class Customer { public string Name { get; set; } = ""; }

    public interface IRepository
    {
        IReadOnlyList<Customer> GetAllCustomers();
    }

    public class ExplicitTypeExample
    {
        private readonly IRepository _repo;
        public ExplicitTypeExample(IRepository repo) => _repo = repo;

        public IReadOnlyList<Customer> GetCustomers()
            => _repo.GetAllCustomers();
    }
}

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24

using System.Collections.Generic;
using Moq;
using Xunit;
using AntiPatterns;

public class ExplicitTypeExampleTests
{
    [Fact]
    public void GetCustomers_ReturnsIReadOnlyList()
    {
        var list = new List<Customer>
        {
            new() { Name = "Test" }
        };
        var repo = new Mock<IRepository>();
        repo.Setup(r => r.GetAllCustomers()).Returns(list);

        var example = new ExplicitTypeExample(repo.Object);
        var result  = example.GetCustomers();

        Assert.IsAssignableFrom<IReadOnlyList<Customer>>(result);
        Assert.Equal(list, result);
    }
}

9️⃣ Magic strings → nameof() for refactor-safe code

1
2
3
4
5
6
7
8
9
10
11
12

namespace AntiPatterns
{
    public static class NameValidator
    {
        public static void SetName(string? name)
        {
            if (name == null)
                throw new ArgumentNullException(nameof(name));
            // assign to field/property…
        }
    }
}

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15

using System;
using Xunit;
using AntiPatterns;

public class NameValidatorTests
{
    [Fact]
    public void SetName_Null_ThrowsWithParamName()
    {
        var ex = Assert.Throws<ArgumentNullException>(
            () => NameValidator.SetName(null!));

        Assert.Equal("name", ex.ParamName);
    }
}

🔟 async void → Only for event handlers—use async Task

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19

namespace AntiPatterns
{
    public interface IDataService { Task<int> GetCountAsync(); }

    public class Counter
    {
        private readonly IDataService _svc;
        public int Value { get; private set; }

        // ✅ returns Task, exceptions propagate normally
        public async Task LoadCountAsync()
        {
            Value = await _svc.GetCountAsync()
                              .ConfigureAwait(false);
        }

        public Counter(IDataService svc) => _svc = svc;
    }
}

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19

using System.Threading.Tasks;
using Moq;
using Xunit;
using AntiPatterns;

public class CounterTests
{
    [Fact]
    public async Task LoadCountAsync_SetsValueCorrectly()
    {
        var mock = new Mock<IDataService>();
        mock.Setup(x => x.GetCountAsync()).ReturnsAsync(42);

        var counter = new Counter(mock.Object);
        await counter.LoadCountAsync();

        Assert.Equal(42, counter.Value);
    }
}

📝 Extended Code Review Checklist

  • Null-safety: use ?./?? or explicit checks
  • String concatenation: no += in loops—prefer StringBuilder
  • Async blocking: no .Result/.Wait()—always await + ConfigureAwait(false)
  • Dispose resources: wrap IDisposable in using or call .Dispose()
  • String comparison: use string.Equals(..., StringComparison)
  • Collection mutation: iterate over .ToList() or use RemoveAll
  • Exception handling: log with context then throw;
  • Type clarity: avoid var when the type isn’t obvious
  • Refactor safety: replace magic strings with nameof()
  • Async signatures: avoid async void except for event handlers

© Mark Norgren. Some rights reserved.

Build Date: 2025-06-06

3f535e3