From 2382a2d90fe2845621f51cada133083bb6adcc94 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 26 Sep 2026 16:27:52 +0000 Subject: [PATCH] fix: accept Nullable element types in the ordered containers without a comparer [patch] The no-comparer constructors of OrderedSet, OrderedCollection and OrderedMap required T itself to implement IComparable or IComparable. Nullable implements neither, so int?, DateTime? and the like were rejected even though Comparer.Default orders them, null first. Move the check into one Comparability.HasDefaultOrdering() helper that falls back to the nullable's underlying type, and use it in every constructor of the three containers. Fixes #71 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01YXTjHR3MfJpmxt8LEwNkjm --- Containers.Test/OrderedCollectionTests.cs | 16 ++++++++++++ Containers.Test/OrderedMapTests.cs | 18 +++++++++++++ Containers.Test/OrderedSetTests.cs | 16 ++++++++++++ Containers/Comparability.cs | 31 +++++++++++++++++++++++ Containers/OrderedCollection.cs | 15 +++-------- Containers/OrderedMap.cs | 13 +++------- Containers/OrderedSet.cs | 15 +++-------- 7 files changed, 90 insertions(+), 34 deletions(-) create mode 100644 Containers/Comparability.cs diff --git a/Containers.Test/OrderedCollectionTests.cs b/Containers.Test/OrderedCollectionTests.cs index 227bb0c..5a0b1c5 100644 --- a/Containers.Test/OrderedCollectionTests.cs +++ b/Containers.Test/OrderedCollectionTests.cs @@ -595,6 +595,22 @@ public void WorksWithStrings_MaintainsAlphabeticalOrder() Assert.AreEqual("zebra", collection[3]); } + [TestMethod] + public void Constructor_WithNullableType_WithoutComparer_SortsNullFirst() + { + OrderedCollection collection = [3, null, 1]; + OrderedCollection withCapacity = new(10) { 2, null }; + OrderedCollection fromCollection = new([2, null, 2]); + + Assert.AreSequenceEqual([null, 1, 3], collection); + Assert.AreSequenceEqual([null, 2], withCapacity); + Assert.AreSequenceEqual([null, 2, 2], fromCollection); + } + + [TestMethod] + public void Constructor_WithNullableOfNonComparableType_WithoutComparer_ThrowsArgumentException() => + Assert.ThrowsExactly(() => new OrderedCollection?>()); + [TestMethod] public void Constructor_WithNonComparableType_WithoutComparer_ThrowsArgumentException() { diff --git a/Containers.Test/OrderedMapTests.cs b/Containers.Test/OrderedMapTests.cs index af0c180..900f601 100644 --- a/Containers.Test/OrderedMapTests.cs +++ b/Containers.Test/OrderedMapTests.cs @@ -420,6 +420,24 @@ public void Values_IsReadOnly() /// /// Tests error handling for constructor with non-comparable type. /// + // OrderedMap constrains TKey to notnull, as Dictionary does, so a nullable key type draws a nullability + // warning at compile time. It must still construct and order keys at run time. +#pragma warning disable CS8714 // Nullability of type argument doesn't match 'notnull' constraint + [TestMethod] + public void Constructor_NullableKeyType_SortsKeys() + { + OrderedMap map = new() { [3] = "three", [1] = "one" }; + OrderedMap withCapacity = new(10) { [5] = "five", [2] = "two" }; + + Assert.AreSequenceEqual([1, 3], map.Keys); + Assert.AreSequenceEqual([2, 5], withCapacity.Keys); + } + + [TestMethod] + public void Constructor_NullableOfNonComparableKeyType_ThrowsArgumentException() => + Assert.ThrowsExactly(() => new OrderedMap?, string>()); +#pragma warning restore CS8714 + [TestMethod] public void Constructor_NonComparableType_ThrowsArgumentException() { diff --git a/Containers.Test/OrderedSetTests.cs b/Containers.Test/OrderedSetTests.cs index c49465d..c9bb53b 100644 --- a/Containers.Test/OrderedSetTests.cs +++ b/Containers.Test/OrderedSetTests.cs @@ -62,6 +62,22 @@ public void Constructor_WithCollectionAndComparer_CreatesSetFromCollectionWithCo Assert.AreSequenceEqual(expected, set); } + [TestMethod] + public void Constructor_NullableTypeWithoutComparer_SortsNullFirst() + { + OrderedSet set = [3, null, 1, null]; + OrderedSet withCapacity = new(10) { 2, null }; + OrderedSet fromCollection = new([2, null, 2]); + + Assert.AreSequenceEqual([null, 1, 3], set); + Assert.AreSequenceEqual([null, 2], withCapacity); + Assert.AreSequenceEqual([null, 2], fromCollection); + } + + [TestMethod] + public void Constructor_NullableOfNonComparableTypeWithoutComparer_ThrowsArgumentException() => + Assert.ThrowsExactly(() => _ = new OrderedSet?>()); + [TestMethod] public void Constructor_NonComparableTypeWithoutComparer_ThrowsArgumentException() { diff --git a/Containers/Comparability.cs b/Containers/Comparability.cs new file mode 100644 index 0000000..a9df3e7 --- /dev/null +++ b/Containers/Comparability.cs @@ -0,0 +1,31 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Containers; + +/// +/// Checks whether a type can be ordered by when no comparer is supplied. +/// +internal static class Comparability +{ + /// + /// Determines whether has a default ordering. + /// + /// + /// A implements neither comparison interface itself, but + /// orders it through its underlying type, with null first, so the underlying type is the one checked. + /// + /// The type to check. + /// if or its underlying type implements or . + internal static bool HasDefaultOrdering() + { + if (typeof(IComparable).IsAssignableFrom(typeof(T)) || typeof(IComparable).IsAssignableFrom(typeof(T))) + { + return true; + } + + Type? underlyingType = Nullable.GetUnderlyingType(typeof(T)); + return underlyingType is not null + && (typeof(IComparable<>).MakeGenericType(underlyingType).IsAssignableFrom(underlyingType) + || typeof(IComparable).IsAssignableFrom(underlyingType)); + } +} diff --git a/Containers/OrderedCollection.cs b/Containers/OrderedCollection.cs index 9a3b63d..f20c587 100644 --- a/Containers/OrderedCollection.cs +++ b/Containers/OrderedCollection.cs @@ -79,10 +79,7 @@ public T this[int index] /// Thrown when T does not implement IComparable{T}. public OrderedCollection() { - if ( - !typeof(IComparable).IsAssignableFrom(typeof(T)) - && !typeof(IComparable).IsAssignableFrom(typeof(T)) - ) + if (!Comparability.HasDefaultOrdering()) { throw new ArgumentException( $"Type {typeof(T)} must implement IComparable or IComparable when no comparer is provided." @@ -116,10 +113,7 @@ public OrderedCollection(int capacity) { ArgumentOutOfRangeException.ThrowIfNegative(capacity); - if ( - !typeof(IComparable).IsAssignableFrom(typeof(T)) - && !typeof(IComparable).IsAssignableFrom(typeof(T)) - ) + if (!Comparability.HasDefaultOrdering()) { throw new ArgumentException( $"Type {typeof(T)} must implement IComparable or IComparable when no comparer is provided." @@ -156,10 +150,7 @@ public OrderedCollection(IEnumerable collection) { Ensure.NotNull(collection); - if ( - !typeof(IComparable).IsAssignableFrom(typeof(T)) - && !typeof(IComparable).IsAssignableFrom(typeof(T)) - ) + if (!Comparability.HasDefaultOrdering()) { throw new ArgumentException( $"Type {typeof(T)} must implement IComparable or IComparable when no comparer is provided." diff --git a/Containers/OrderedMap.cs b/Containers/OrderedMap.cs index 8fa4af7..39759d1 100644 --- a/Containers/OrderedMap.cs +++ b/Containers/OrderedMap.cs @@ -53,8 +53,7 @@ private struct Entry(TKey key, TValue value) /// private readonly List items = comparer is null - && !typeof(IComparable).IsAssignableFrom(typeof(TKey)) - && !typeof(IComparable).IsAssignableFrom(typeof(TKey)) + && !Comparability.HasDefaultOrdering() ? throw new ArgumentException( $"Type {typeof(TKey)} must implement IComparable or IComparable when no comparer is provided." ) @@ -145,10 +144,7 @@ public OrderedMap(int capacity) { ArgumentOutOfRangeException.ThrowIfNegative(capacity); - if ( - !typeof(IComparable).IsAssignableFrom(typeof(TKey)) - && !typeof(IComparable).IsAssignableFrom(typeof(TKey)) - ) + if (!Comparability.HasDefaultOrdering()) { throw new ArgumentException( $"Type {typeof(TKey)} must implement IComparable or IComparable when no comparer is provided." @@ -187,10 +183,7 @@ public OrderedMap(IDictionary dictionary) { Ensure.NotNull(dictionary); - if ( - !typeof(IComparable).IsAssignableFrom(typeof(TKey)) - && !typeof(IComparable).IsAssignableFrom(typeof(TKey)) - ) + if (!Comparability.HasDefaultOrdering()) { throw new ArgumentException( $"Type {typeof(TKey)} must implement IComparable or IComparable when no comparer is provided." diff --git a/Containers/OrderedSet.cs b/Containers/OrderedSet.cs index 62de21d..3af2841 100644 --- a/Containers/OrderedSet.cs +++ b/Containers/OrderedSet.cs @@ -69,10 +69,7 @@ public class OrderedSet : ISet /// Thrown when T does not implement IComparable{T}. public OrderedSet() { - if ( - !typeof(IComparable).IsAssignableFrom(typeof(T)) - && !typeof(IComparable).IsAssignableFrom(typeof(T)) - ) + if (!Comparability.HasDefaultOrdering()) { throw new ArgumentException( $"Type {typeof(T)} must implement IComparable or IComparable when no comparer is provided." @@ -106,10 +103,7 @@ public OrderedSet(int capacity) { ArgumentOutOfRangeException.ThrowIfNegative(capacity); - if ( - !typeof(IComparable).IsAssignableFrom(typeof(T)) - && !typeof(IComparable).IsAssignableFrom(typeof(T)) - ) + if (!Comparability.HasDefaultOrdering()) { throw new ArgumentException( $"Type {typeof(T)} must implement IComparable or IComparable when no comparer is provided." @@ -146,10 +140,7 @@ public OrderedSet(IEnumerable collection) { Ensure.NotNull(collection); - if ( - !typeof(IComparable).IsAssignableFrom(typeof(T)) - && !typeof(IComparable).IsAssignableFrom(typeof(T)) - ) + if (!Comparability.HasDefaultOrdering()) { throw new ArgumentException( $"Type {typeof(T)} must implement IComparable or IComparable when no comparer is provided."