From acc2470f80fa9a09defa109a5e691e7ba1b41b9c Mon Sep 17 00:00:00 2001 From: MrDave1999 Date: Tue, 28 Jul 2026 11:48:56 -0500 Subject: [PATCH] refactor(map)!: use FrozenDictionary for map definitions Replace the mutable dictionary of map definitions with FrozenDictionary. The registered dictionaries are created only once during the first resolution and remain alive for the lifetime of the application. As the collection is immutable after construction, FrozenDictionary better represents its intended usage through the type system and optimizes lookups for scenarios with a large number of registered map definitions. BREAKING CHANGE: Dictionary is no longer registered in the dependency injection container. Consumers should depend on FrozenDictionary instead. --- .../MapObjectServiceTests.cs | 42 ++++++------------- src/SampSharp.MapObjects.Tests/Usings.cs | 1 + src/SampSharp.MapObjects/MapObjectService.cs | 2 +- .../ServiceCollectionExtensions.cs | 2 +- src/SampSharp.MapObjects/Usings.cs | 1 + 5 files changed, 17 insertions(+), 31 deletions(-) diff --git a/src/SampSharp.MapObjects.Tests/MapObjectServiceTests.cs b/src/SampSharp.MapObjects.Tests/MapObjectServiceTests.cs index 25d3027..bbf238d 100644 --- a/src/SampSharp.MapObjects.Tests/MapObjectServiceTests.cs +++ b/src/SampSharp.MapObjects.Tests/MapObjectServiceTests.cs @@ -3,7 +3,6 @@ public class MapObjectServiceTests { private FakeMapContext _fakeMapContext; - private Dictionary _maps; [SetUp] public void Init() @@ -13,15 +12,13 @@ public void Init() Substitute.For(), NullLogger.Instance ); - - _maps = []; } [Test] public void Load_WhenMapNameIsNull_ShouldThrowArgumentNullException() { // Arrange - MapObjectService mapObjects = new(_fakeMapContext, _maps); + MapObjectService mapObjects = new(_fakeMapContext, CreateMaps()); // Act Action act = () => mapObjects.Load(null!); @@ -34,7 +31,7 @@ public void Load_WhenMapNameIsNull_ShouldThrowArgumentNullException() public void Load_WhenMapNameIsEmpty_ShouldThrowArgumentException() { // Arrange - MapObjectService mapObjects = new(_fakeMapContext, _maps); + MapObjectService mapObjects = new(_fakeMapContext, CreateMaps()); // Act Action act = () => mapObjects.Load(string.Empty); @@ -47,7 +44,7 @@ public void Load_WhenMapNameIsEmpty_ShouldThrowArgumentException() public void Load_WhenMapIsNotRegistered_ShouldDoNothing() { // Arrange - MapObjectService mapObjects = new(_fakeMapContext, _maps); + MapObjectService mapObjects = new(_fakeMapContext, CreateMaps()); // Act Action act = () => mapObjects.Load("Unknown"); @@ -61,9 +58,7 @@ public void Load_WhenMapIsRegistered_ShouldLoadMap() { // Arrange TestMapDefinition map = new("Map"); - _maps.Add(map.Name, map); - - MapObjectService mapObjects = new(_fakeMapContext, _maps); + MapObjectService mapObjects = new(_fakeMapContext, CreateMaps(map)); // Act mapObjects.Load(map.Name); @@ -78,11 +73,7 @@ public void Load_WhenAnotherMapIsAlreadyLoaded_ShouldThrowInvalidOperationExcept // Arrange TestMapDefinition map1 = new("Map1"); TestMapDefinition map2 = new("Map2"); - - _maps.Add(map1.Name, map1); - _maps.Add(map2.Name, map2); - - MapObjectService mapObjects = new(_fakeMapContext, _maps); + MapObjectService mapObjects = new(_fakeMapContext, CreateMaps(map1, map2)); mapObjects.Load(map1.Name); // Act @@ -96,7 +87,7 @@ public void Load_WhenAnotherMapIsAlreadyLoaded_ShouldThrowInvalidOperationExcept public void Unload_WhenNoMapIsLoaded_ShouldThrowInvalidOperationException() { // Arrange - MapObjectService mapObjects = new(_fakeMapContext, _maps); + MapObjectService mapObjects = new(_fakeMapContext, CreateMaps()); // Act Action act = mapObjects.Unload; @@ -110,9 +101,7 @@ public void Unload_WhenMapIsLoaded_ShouldUnloadMap() { // Arrange TestMapDefinition map = new("Map"); - _maps.Add(map.Name, map); - - MapObjectService mapObjects = new(_fakeMapContext, _maps); + MapObjectService mapObjects = new(_fakeMapContext, CreateMaps(map)); mapObjects.Load(map.Name); // Act @@ -129,11 +118,7 @@ public void Unload_WhenMapWasUnloaded_ShouldAllowLoadingAnotherMap() // Arrange TestMapDefinition map1 = new("Map1"); TestMapDefinition map2 = new("Map2"); - - _maps.Add(map1.Name, map1); - _maps.Add(map2.Name, map2); - - MapObjectService mapObjects = new(_fakeMapContext, _maps); + MapObjectService mapObjects = new(_fakeMapContext, CreateMaps(map1, map2)); mapObjects.Load(map1.Name); mapObjects.Unload(); @@ -153,13 +138,8 @@ public void Unload_WhenOnUnloadThrows_ShouldAllowLoadingAnotherMap() { ThrowOnUnload = true }; - TestMapDefinition map2 = new("Map2"); - - _maps.Add(map1.Name, map1); - _maps.Add(map2.Name, map2); - - MapObjectService mapObjects = new(_fakeMapContext, _maps); + MapObjectService mapObjects = new(_fakeMapContext, CreateMaps(map1, map2)); mapObjects.Load(map1.Name); // Act @@ -172,4 +152,8 @@ public void Unload_WhenOnUnloadThrows_ShouldAllowLoadingAnotherMap() load.Should().NotThrow(); map2.WasLoaded.Should().BeTrue(); } + + private static FrozenDictionary CreateMaps( + params MapDefinition[] maps) + => maps.ToFrozenDictionary(m => m.Name); } diff --git a/src/SampSharp.MapObjects.Tests/Usings.cs b/src/SampSharp.MapObjects.Tests/Usings.cs index 1cd0943..48f7202 100644 --- a/src/SampSharp.MapObjects.Tests/Usings.cs +++ b/src/SampSharp.MapObjects.Tests/Usings.cs @@ -1,4 +1,5 @@ global using System.Numerics; +global using System.Collections.Frozen; global using NUnit.Framework; global using FluentAssertions; global using NSubstitute; diff --git a/src/SampSharp.MapObjects/MapObjectService.cs b/src/SampSharp.MapObjects/MapObjectService.cs index 4e95186..948fe0c 100644 --- a/src/SampSharp.MapObjects/MapObjectService.cs +++ b/src/SampSharp.MapObjects/MapObjectService.cs @@ -2,7 +2,7 @@ internal sealed class MapObjectService( MapContext mapContext, - Dictionary maps) : IMapObjectService + FrozenDictionary maps) : IMapObjectService { private MapDefinition _loadedMap; diff --git a/src/SampSharp.MapObjects/ServiceCollectionExtensions.cs b/src/SampSharp.MapObjects/ServiceCollectionExtensions.cs index 0995200..20a3929 100644 --- a/src/SampSharp.MapObjects/ServiceCollectionExtensions.cs +++ b/src/SampSharp.MapObjects/ServiceCollectionExtensions.cs @@ -59,7 +59,7 @@ public static IServiceCollection AddMapObjects(this IServiceCollection services) .AddSingleton(sp => { IEnumerable maps = sp.GetRequiredService>(); - return maps.ToDictionary(map => map.Name); + return maps.ToFrozenDictionary(map => map.Name); }) .AddSingleton() .AddSingleton() diff --git a/src/SampSharp.MapObjects/Usings.cs b/src/SampSharp.MapObjects/Usings.cs index 3015407..af9bbce 100644 --- a/src/SampSharp.MapObjects/Usings.cs +++ b/src/SampSharp.MapObjects/Usings.cs @@ -1,4 +1,5 @@ global using System.Numerics; +global using System.Collections.Frozen; global using Microsoft.Extensions.Logging; global using Microsoft.Extensions.DependencyInjection; global using SampSharp.Entities;