From 1e1ca5519cbe252c8949bc18d5a248483e0b73ce Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 24 May 2026 14:31:20 -0400 Subject: [PATCH] NEO-59: address code review follow-ups Simplify GetDefinitionsInIdOrder to iterate key-value pairs directly, avoiding the intermediate sorted-keys array and second dictionary lookup. --- docs/reviews/2026-05-24-NEO-59.md | 16 ++++++++-------- .../Gathering/ResourceNodeDefinitionRegistry.cs | 16 +++++----------- 2 files changed, 13 insertions(+), 19 deletions(-) diff --git a/docs/reviews/2026-05-24-NEO-59.md b/docs/reviews/2026-05-24-NEO-59.md index 3c2f2f5..90d4050 100644 --- a/docs/reviews/2026-05-24-NEO-59.md +++ b/docs/reviews/2026-05-24-NEO-59.md @@ -33,16 +33,16 @@ None. ## Nits -- `GetDefinitionsInIdOrder()` iterates `.Keys` to build a sorted array then does a second dictionary lookup per key. Iterating key-value pairs directly is slightly more idiomatic and avoids the double lookup: +- ~~`GetDefinitionsInIdOrder()` iterates `.Keys` to build a sorted array then does a second dictionary lookup per key. Iterating key-value pairs directly is slightly more idiomatic and avoids the double lookup:~~ - ```csharp - return catalog.NodesById - .OrderBy(kv => kv.Key, StringComparer.Ordinal) - .Select(kv => kv.Value) - .ToList(); - ``` + ~~```csharp~~ + ~~return catalog.NodesById~~ + ~~ .OrderBy(kv => kv.Key, StringComparer.Ordinal)~~ + ~~ .Select(kv => kv.Value)~~ + ~~ .ToList();~~ + ~~```~~ - The current form matches `ItemDefinitionRegistry` exactly, so consistency may outweigh the change. No action required. + ~~The current form matches `ItemDefinitionRegistry` exactly, so consistency may outweigh the change. No action required.~~ Done. - The null-id and unknown-id tests each build a fully populated `nodes`/`yields` dictionary even though the exercised code paths exit before those entries are touched. Consistent with `ItemDefinitionRegistryTests` — no action required. diff --git a/server/NeonSprawl.Server/Game/Gathering/ResourceNodeDefinitionRegistry.cs b/server/NeonSprawl.Server/Game/Gathering/ResourceNodeDefinitionRegistry.cs index 07aab07..0c41dae 100644 --- a/server/NeonSprawl.Server/Game/Gathering/ResourceNodeDefinitionRegistry.cs +++ b/server/NeonSprawl.Server/Game/Gathering/ResourceNodeDefinitionRegistry.cs @@ -44,15 +44,9 @@ public sealed class ResourceNodeDefinitionRegistry(ResourceNodeCatalog catalog) } /// - public IReadOnlyList GetDefinitionsInIdOrder() - { - var ids = catalog.NodesById.Keys.OrderBy(k => k, StringComparer.Ordinal).ToArray(); - var list = new List(ids.Length); - foreach (var id in ids) - { - list.Add(catalog.NodesById[id]); - } - - return list; - } + public IReadOnlyList GetDefinitionsInIdOrder() => + catalog.NodesById + .OrderBy(kv => kv.Key, StringComparer.Ordinal) + .Select(kv => kv.Value) + .ToList(); }