Minecraft-Console-Client/.skills/csharp-best-practices/SKILL.md

21 KiB
Raw Blame History

name description version
csharp-best-practices C# 12 / .NET 8 coding conventions, idiomatic patterns, and performance best practices for the Minecraft Console Client codebase. Use when writing, reviewing, or modifying C# code. 0.3.0

C# 12 / .NET 8 Best Practices

Target: .NET 8, C# 12, nullable enabled. Sources: MS C# Conventions · .NET Runtime Style · C# 12 Docs · .NET 8 Perf

Naming

Element Style Example
Type, method, property, const, enum member PascalCase PacketHandler, MaxRetries, GameMode.Survival
Interface I + PascalCase IChatBot
Private instance field _camelCase _handler
Private static field s_camelCase s_defaultTimeout
Thread-static field t_camelCase t_cachedBuffer
Local, parameter camelCase packetId
Type parameter T + PascalCase TResult
Namespace PascalCase MinecraftClient.Protocol
Async methods Suffix Async ConnectAsync(), ReadPacketAsync()
// CORRECT: naming conventions
private readonly Dictionary<int, Entity> _entities = new();
private static readonly TimeSpan s_reconnectDelay = TimeSpan.FromSeconds(5);
public int PacketCount { get; private set; }
public async Task<bool> ConnectAsync(CancellationToken ct) { }
// WRONG: naming violations
private Dictionary<int, Entity> entities = new();   // missing _
private static TimeSpan reconnectDelay;              // missing s_
public int packet_count { get; set; }                // snake_case
public async Task<bool> Connect(CancellationToken ct) { }  // missing Async suffix

C# 12 Features

Primary Constructors

Use for simple parameter capture. Parameters are camelCase, mutable — assign to readonly fields when immutability matters.

// CORRECT: primary constructor captures dependencies
public class ChatLogger(string logFilePath, bool appendMode) : ChatBot
{
    private readonly StreamWriter _writer = new(logFilePath, appendMode);
    public override void GetText(string text) => _writer.WriteLine(text);
}
// WRONG: verbose constructor boilerplate for simple capture
public class ChatLogger : ChatBot
{
    private readonly StreamWriter _writer;
    public ChatLogger(string logFilePath, bool appendMode)
    {
        _writer = new StreamWriter(logFilePath, appendMode);
    }
    public override void GetText(string text) => _writer.WriteLine(text);
}

Collection Expressions

Use [...] and .. spread for arrays, lists, spans.

// CORRECT: collection expressions (C# 12)
int[] ids = [1, 2, 3];
List<string> names = ["Steve", "Alex"];
ReadOnlySpan<byte> header = [0xFE, 0x01];           // no heap alloc
int[] combined = [..firstArray, ..secondArray, 42];
IReadOnlyList<string> empty = [];
// WRONG: verbose initialization
int[] ids = new int[] { 1, 2, 3 };
var names = new List<string> { "Steve", "Alex" };
ReadOnlySpan<byte> header = new byte[] { 0xFE, 0x01 };  // allocates
var combined = firstArray.Concat(secondArray).Append(42).ToArray();

Type Aliases

// CORRECT: alias complex types for readability
using Coordinate = (int X, int Y, int Z);
using PacketMap = System.Collections.Generic.Dictionary<int, System.Action<byte[]>>;

Default Lambda Parameters

// CORRECT: C# 12
var greet = (string name, string prefix = "Player") => $"{prefix} {name}";

Modern Syntax (C# 1012)

File-Scoped Namespaces

// CORRECT: file-scoped namespace — one per file, less nesting
namespace MinecraftClient.ChatBots;

public class MyBot : ChatBot { }
// WRONG: block-scoped namespace adds unnecessary nesting
namespace MinecraftClient.ChatBots
{
    public class MyBot : ChatBot { }
}

Target-Typed new

Use when the type is obvious from the left-hand side.

// CORRECT: target-typed new
private readonly Dictionary<string, int> _scores = new();
List<Entity> entities = new(capacity: 256);
// WRONG: redundant type name
private readonly Dictionary<string, int> _scores = new Dictionary<string, int>();

Pattern Matching

Prefer patterns over type-casting chains and complex boolean logic.

// CORRECT: is-pattern with declaration and property patterns
if (entity is Player { Health: > 0 } player)
    SendMessage($"{player.Name} is alive");
// WRONG: manual cast and multi-step check
if (entity is Player)
{
    var player = (Player)entity;
    if (player.Health > 0)
        SendMessage($"{player.Name} is alive");
}
// CORRECT: switch expression
public string GetStatusLabel(GameMode mode) => mode switch
{
    GameMode.Survival  => "Survival",
    GameMode.Creative  => "Creative",
    GameMode.Adventure => "Adventure",
    GameMode.Spectator => "Spectator",
    _ => throw new ArgumentOutOfRangeException(nameof(mode))
};
// WRONG: switch statement with returns
public string GetStatusLabel(GameMode mode)
{
    switch (mode)
    {
        case GameMode.Survival: return "Survival";
        case GameMode.Creative: return "Creative";
        default: throw new ArgumentOutOfRangeException(nameof(mode));
    }
}
// CORRECT: property patterns for compound conditions
if (response is { StatusCode: >= 200 and < 300, Content.Length: > 0 })
    ProcessResponse(response);
// WRONG: multiple chained conditions
if (response != null && response.StatusCode >= 200
    && response.StatusCode < 300 && response.Content != null
    && response.Content.Length > 0)
    ProcessResponse(response);
// CORRECT: relational, logical, and list patterns
if (health is > 0 and <= 6) LogToConsole("Low health!");
if (args is [var command, var target, ..]) ProcessCommand(command, target);

Raw String Literals

Use for JSON, regex, multi-line strings.

// CORRECT: raw string literal
string json = """
    { "username": "Steve", "action": "connect" }
    """;
string pattern = """<\w+>""";
// WRONG: escaped quotes
string json = "{ \"username\": \"Steve\", \"action\": \"connect\" }";

Records

Use record for immutable data carriers and DTOs. Use record struct for small value types.

// CORRECT: record for data carrier
public record PlayerInfo(string Name, Guid Uuid, GameMode Mode);
public record struct ChunkCoord(int X, int Z);
var updated = info with { Mode = GameMode.Creative };
// WRONG: full class for a simple data carrier
public class PlayerInfo
{
    public string Name { get; set; } = string.Empty;
    public Guid Uuid { get; set; }
    public GameMode Mode { get; set; }
}
// CORRECT: compact constructor for record validation
public record OrderItem(string ProductId, int Quantity, decimal UnitPrice)
{
    public OrderItem
    {
        ArgumentException.ThrowIfNullOrWhiteSpace(ProductId);
        ArgumentOutOfRangeException.ThrowIfNegativeOrZero(Quantity);
        ArgumentOutOfRangeException.ThrowIfNegative(UnitPrice);
    }
}

Required Members

// CORRECT: required + init enforces initialization without constructor boilerplate
public class ServerConfig
{
    public required string Host { get; init; }
    public required int Port { get; init; }
    public string? Password { get; init; }
}
var config = new ServerConfig { Host = "mc.example.com", Port = 25565 };

Nullable Reference Types

Project has nullable enabled. Follow these rules:

// CORRECT: guard at API boundaries with .NET 8 throw helpers
public void Connect(string host, IProtocolHandler handler)
{
    ArgumentNullException.ThrowIfNull(handler);
    ArgumentException.ThrowIfNullOrEmpty(host);
}
// WRONG: manual null checks
if (handler == null) throw new ArgumentNullException(nameof(handler));
if (string.IsNullOrWhiteSpace(host))
    throw new ArgumentException("Host is required", nameof(host));
// CORRECT: 'is not null' pattern
if (currentPlayer is not null)
    currentPlayer.Update();
// WRONG: comparison operator for null check
if (currentPlayer != null)
    currentPlayer.Update();
// CORRECT: explicit nullable handling
public Entity? FindEntity(int id)
{
    return _entities.TryGetValue(id, out var entity) ? entity : null;
}

// CORRECT: null-coalescing / null-conditional
string name = player?.CustomName ?? player?.Name ?? "Unknown";

// CORRECT: null-forgiving only when proven safe (after ThrowIfNull or equivalent)
string val = GetRequiredValue()!;

// CORRECT: annotate return values
[return: MaybeNull]
public T Find<T>(Predicate<T> match) { }

[MemberNotNull(nameof(_connection))]
private void EnsureConnected() { }
// WRONG: hiding nullability with null-forgiving
public string GetName(Player? player)
{
    return player!.Name;  // hides potential NullReferenceException
}

Async / Await

// CORRECT: propagate CancellationToken through every async I/O call
public async Task<string> FetchDataAsync(Uri uri, CancellationToken ct = default)
{
    using var response = await _httpClient.GetAsync(uri, ct);
    return await response.Content.ReadAsStringAsync(ct);
}
// WRONG: CancellationToken not passed downstream
public async Task<string> FetchDataAsync(Uri uri)
{
    using var response = await _httpClient.GetAsync(uri, default);
    return await response.Content.ReadAsStringAsync(default);
}
// CORRECT: ValueTask when result is often available synchronously
public ValueTask<int> GetCachedCountAsync()
{
    if (_cache.TryGetValue("count", out int count))
        return ValueTask.FromResult(count);
    return new ValueTask<int>(LoadCountFromDbAsync());
}
// WRONG: Task allocates unnecessarily when result is cached
public async Task<int> GetCachedCountAsync()
{
    if (_cache.TryGetValue("count", out int count))
        return count;  // allocates a Task
    return await LoadCountFromDbAsync();
}
// CORRECT: async Task for async event handlers
public async Task HandleEventAsync(GameEvent e, CancellationToken ct)
{
    await notificationService.SendAsync(e.PlayerId, ct);
}
// WRONG: async void — exceptions are unobservable, cannot be awaited
public async void HandleEvent(GameEvent e)
{
    await notificationService.SendAsync(e.PlayerId, default);
}
// CORRECT: await the result
var packet = await reader.ReadPacketAsync(ct);
// WRONG: .Result / .Wait() causes deadlocks
var packet = reader.ReadPacketAsync(ct).Result;
var packet2 = reader.ReadPacketAsync(ct).GetAwaiter().GetResult();
// CORRECT: ConfigureAwait(false) in library code
var data = await stream.ReadAsync(buffer, ct).ConfigureAwait(false);

// CORRECT: IAsyncEnumerable for streaming
public async IAsyncEnumerable<ChatMessage> ReadChatStreamAsync(
    [EnumeratorCancellation] CancellationToken ct = default)
{
    while (!ct.IsCancellationRequested)
        yield return await _reader.ReadNextAsync(ct);
}

// CORRECT: await using for async disposal
await using var conn = new McConnection(host, port);

LINQ

Prefer Method Syntax for Most Operations

// CORRECT: method syntax for common operations
var onlinePlayers = players
    .Where(p => p.IsOnline)
    .OrderBy(p => p.Name)
    .Select(p => new PlayerListItem(p.Id, p.Name))
    .ToList();
// AVOID: query syntax for simple operations
var onlinePlayers = (
    from p in players
    where p.IsOnline
    orderby p.Name
    select new PlayerListItem(p.Id, p.Name)
).ToList();

Use Query Syntax for Joins

// CORRECT: query syntax makes joins readable
var results =
    from entity in entities
    join player in players on entity.OwnerId equals player.Id
    where entity.Health > 0
    select new { entity.Name, player.Name };
// AVOID: method syntax for complex joins is hard to read
var results = entities
    .Join(players,
        e => e.OwnerId,
        p => p.Id,
        (e, p) => new { e, p })
    .Where(x => x.e.Health > 0)
    .Select(x => new { x.e.Name, PlayerName = x.p.Name });

Materialize to Avoid Multiple Enumeration

// CORRECT: materialize once, iterate many times
var online = players.Where(p => p.IsOnline).ToList();
Console.WriteLine(online.Count);
foreach (var p in online) { }
// WRONG: enumerates the query twice
var filtered = players.Where(p => p.IsOnline);
Console.WriteLine(filtered.Count());     // first enumeration
foreach (var p in filtered) { }          // second enumeration

Use Any() Over Count() > 0

// CORRECT: short-circuits on first match
if (entities.Any(e => e.IsHostile))
    TriggerAlert();
// WRONG: counts the entire collection
if (entities.Count(e => e.IsHostile) > 0)
    TriggerAlert();

Prefer FirstOrDefault with Null Handling

// CORRECT: explicit null handling
var target = players.FirstOrDefault(p => p.Name == name)
    ?? throw new InvalidOperationException($"Player '{name}' not found");

TryGetNonEnumeratedCount

// CORRECT: avoid full enumeration just to get count (.NET 6+)
if (source.TryGetNonEnumeratedCount(out int count))
    buffer = new Entity[count];

Avoid LINQ in Hot Paths

// CORRECT: manual loop with Span in performance-critical code
Span<byte> data = stackalloc byte[256];
int found = 0;
for (int i = 0; i < data.Length; i++)
    if (data[i] == target) found++;
// AVOID: LINQ allocates enumerators and delegates on hot paths
int found = data.ToArray().Count(b => b == target);

Performance (.NET 8)

Span<T> / Memory<T>

// CORRECT: zero-allocation slicing
ReadOnlySpan<char> command = input.AsSpan()[1..];  // skip '/'

// CORRECT: stack-allocated parsing
public static int ParseVarInt(ReadOnlySpan<byte> data, out int bytesRead)
{
    int result = 0; bytesRead = 0; byte cur;
    do { cur = data[bytesRead]; result |= (cur & 0x7F) << (bytesRead * 7); bytesRead++; }
    while ((cur & 0x80) != 0);
    return result;
}

FrozenDictionary / FrozenSet (.NET 8)

Build once, read many — ~50% faster lookups than Dictionary.

// CORRECT: FrozenDictionary for read-heavy lookup tables (palettes, protocol maps)
using System.Collections.Frozen;
private static readonly FrozenDictionary<int, string> s_blockNames =
    new Dictionary<int, string> { [0] = "air", [1] = "stone" }.ToFrozenDictionary();

SearchValues<T> (.NET 8)

Hardware-accelerated set search.

// CORRECT: precompute once, scan with SIMD
private static readonly SearchValues<char> s_separators = SearchValues.Create(" \t\n\r,;");
int idx = input.AsSpan().IndexOfAny(s_separators);

CompositeFormat (.NET 8)

Parse format string once, reuse.

// CORRECT: avoids re-parsing the format string each call
private static readonly CompositeFormat s_logFmt = CompositeFormat.Parse("[{0:HH:mm:ss}] {1}: {2}");
string msg = string.Format(CultureInfo.InvariantCulture, s_logFmt, DateTime.Now, player, text);

ArrayPool / stackalloc

// CORRECT: rent from pool for temporary buffers
byte[] buf = ArrayPool<byte>.Shared.Rent(4096);
try { int n = stream.Read(buf.AsSpan(0, 4096)); ProcessPacket(buf.AsSpan(0, n)); }
finally { ArrayPool<byte>.Shared.Return(buf); }

// CORRECT: stackalloc for small, fixed-size buffers (< 512 bytes)
Span<byte> header = stackalloc byte[5];

String Handling

// CORRECT: explicit StringComparison — always
bool match = name.Equals("Steve", StringComparison.OrdinalIgnoreCase);
int idx = text.IndexOf("hello", StringComparison.Ordinal);
// WRONG: allocates a lowered copy
bool match = name.ToLower() == "steve";
// CORRECT: string.Create for perf-critical formatting
string hex = string.Create(data.Length * 2, data, static (span, bytes) =>
{
    for (int i = 0; i < bytes.Length; i++)
        bytes[i].TryFormat(span[(i * 2)..], out _, "X2");
});

// CORRECT: StringBuilder for loops
var sb = new StringBuilder(256);
foreach (var item in inventory)
    sb.Append(item.Name).Append(" x").Append(item.Count).AppendLine();
// WRONG: O(n²) string concatenation in loop
string combined = "";
foreach (var s in items) combined += s + ", ";

Collections — Choosing the Right Type

Scenario Type Notes
General key-value Dictionary<K,V> O(1) lookup
Build once, read many FrozenDictionary<K,V> .NET 8; faster reads
Thread-safe ConcurrentDictionary<K,V> Lock-free reads
Immutable snapshots ImmutableDictionary<K,V> Persistent structure
Membership test HashSet<T> / FrozenSet<T> FrozenSet for static
Priority queue PriorityQueue<E,P> .NET 6+
Producer-consumer Channel<T> Over BlockingCollection<T>
Temp buffer ArrayPool<T> / stackalloc Zero/low alloc

Error Handling

// CORRECT: Try* pattern for expected failures
if (int.TryParse(input, out int value)) ProcessValue(value);
if (_registry.TryGetValue(packetId, out var handler)) handler.Invoke(data);
// WRONG: using exceptions for control flow
try { return dict[key]; }
catch (KeyNotFoundException) { return null; }  // use TryGetValue
// CORRECT: exception filters (catch-when)
try { await ConnectAsync(ct); }
catch (SocketException ex) when (ex.SocketErrorCode == SocketError.ConnectionRefused)
{
    LogToConsole("Connection refused, retrying...");
}
// CORRECT: throw helpers (smaller IL, better inlining)
ArgumentNullException.ThrowIfNull(handler);
ArgumentOutOfRangeException.ThrowIfNegative(timeout);
ArgumentOutOfRangeException.ThrowIfGreaterThan(timeout, MaxTimeout);
ObjectDisposedException.ThrowIf(_disposed, this);
// WRONG: generic exceptions
throw new Exception($"Entity {id} not found");
// CORRECT: specific, meaningful exception types
throw new EntityNotFoundException(id);
// or use null-coalescing with throw
return await FindEntityAsync(id, ct)
    ?? throw new EntityNotFoundException(id);
// AVOID: catching Exception without filtering
try { DoWork(); }
catch (Exception) { /* swallowed */ }

Warning Suppression

// CORRECT: fix the warning by handling null properly
public string GetDisplayName(Player? player)
{
    return player?.DisplayName ?? "Unknown";
}
// WRONG: suppressing nullable warning with pragma
#pragma warning disable CS8602
public string GetDisplayName(Player? player)
{
    return player.DisplayName;  // NullReferenceException at runtime
}
#pragma warning restore CS8602
// WRONG: suppressing with attribute
[SuppressMessage("Usage", "CA1062:Validate arguments of public methods")]
public void Process(Packet packet)
{
    // missing null check
}

Project-wide .editorconfig is the only acceptable place for warning policy:

# .editorconfig - project-wide policy decisions only
dotnet_diagnostic.CA2007.severity = none

Resource Management

// CORRECT: using declaration — disposed at end of scope
using var stream = new FileStream(path, FileMode.Open);
using var reader = new StreamReader(stream);

// CORRECT: IAsyncDisposable
await using var conn = await CreateConnectionAsync();
// CORRECT: Dispose pattern
public class PacketReader : IDisposable
{
    private Stream? _stream;
    private bool _disposed;
    public void Dispose()
    {
        if (_disposed) return;
        _stream?.Dispose(); _stream = null; _disposed = true;
    }
}

Security

// CORRECT: secure random for tokens
byte[] token = RandomNumberGenerator.GetBytes(32);

// CORRECT: constant-time comparison for secrets
bool valid = CryptographicOperations.FixedTimeEquals(expected, actual);

// CORRECT: validate external input
if (Uri.TryCreate(userInput, UriKind.Absolute, out var uri)
    && uri.Scheme is "http" or "https")
    await FetchAsync(uri);
// WRONG: predictable random for security-sensitive values
var rng = new Random();

// WRONG: timing side-channel on secret comparison
bool eq = secret1.SequenceEqual(secret2);

Miscellaneous Idioms

// CORRECT: var when type is obvious from RHS
var entities = new Dictionary<int, Entity>();
var timer = Stopwatch.StartNew();

// CORRECT: explicit type when var would be unclear
Stream responseStream = GetResponse();
int count = items.Count;

// CORRECT: expression-bodied members for one-liners
public override string ToString() => $"[{X}, {Y}, {Z}]";
public bool IsAlive => Health > 0;

// CORRECT: discards for unused values
_ = int.TryParse(s, out int result);
(_, int y, _) = GetCoordinates();

// CORRECT: nameof for resilient refactoring
throw new ArgumentException("Invalid value", nameof(packetId));
LogToConsole($"{nameof(AutoEat)}: eating {item.Name}");

// CORRECT: static lambdas prevent accidental closure allocations
list.Sort(static (a, b) => a.Id.CompareTo(b.Id));

// CORRECT: index/range operators
var last = items[^1];
var slice = data[3..^1];

// CORRECT: tuple deconstruction
var (x, y, z) = GetPosition();

// CORRECT: string interpolation with alignment and format specifiers
LogToConsole($"Health: {health,6:F1} | Hunger: {hunger,6:F1}");