Fix issue #465. Added tests and adjusted logic to get more complete test coverage (#530)

Author: kamikkels

Algorithms.Tests/Sorters/Comparison/TimSorterTests.cs

@@ -9,8 +9,8 @@ public static class TimSorterTests
     private static readonly TimSorterSettings Settings = new();
 
     [Test]
-    public static void ArraySorted(
-        [Random(0, 10_000, 2000)] int n)
+    public static void Sort_ShouldBeEquivalentToSuccessfulBasicSort(
+        [Random(0, 10_000, 5000)] int n)
     {
         // Arrange
         var sorter = new TimSorter<int>(Settings, IntComparer);
@@ -25,7 +25,7 @@ public static void ArraySorted(
     }
 
     [Test]
-    public static void TinyArray()
+    public static void Sort_TinyArray_ShouldSortCorrectly()
     {
         // Arrange
         var sorter = new TimSorter<int>(Settings, IntComparer);
@@ -40,7 +40,7 @@ public static void TinyArray()
     }
 
     [Test]
-    public static void SmallChunks()
+    public static void Sort_SmallChunks_ShouldSortCorrectly()
     {
         // Arrange
         var sorter = new TimSorter<int>(Settings, IntComparer);
@@ -63,4 +63,74 @@ public static void SmallChunks()
         // Assert
         Assert.That(correctArray, Is.EqualTo(testArray));
     }
+
+    [Test]
+    public static void Sort_ThrowsArgumentNullException_WhenArrayIsNull()
+    {
+        // Arrange
+        var sorter = new TimSorter<int>(Settings, IntComparer);
+
+        // Act & Assert
+        Assert.Throws<ArgumentNullException>(() => sorter.Sort(null!, IntComparer));
+    }
+
+    [Test]
+    public static void Sort_UsesDefaultComparer_WhenComparerIsNull()
+    {
+        // Arrange
+        var sorter = new TimSorter<int>(Settings, null!);
+        var (correctArray, testArray) = RandomHelper.GetArrays(20);
+
+        // Act
+        sorter.Sort(testArray, IntComparer);
+        Array.Sort(correctArray, IntComparer);
+
+        // Assert
+        Assert.That(correctArray, Is.EqualTo(testArray));
+    }
+
+    [Test]
+    public static void Sort_AlreadySortedArray_RemainsUnchanged()
+    {
+        // Arrange
+        var sorter = new TimSorter<int>(Settings, IntComparer);
+        var array = new[] { 1, 2, 3, 4, 5 };
+        var expected = new[] { 1, 2, 3, 4, 5 };
+
+        // Act
+        sorter.Sort(array, IntComparer);
+
+        // Assert
+        Assert.That(array, Is.EqualTo(expected));
+    }
+
+    [Test]
+    public static void MergeAt_ShouldReturnEarly_WhenLenAIsZero()
+    {
+        // Arrange: left run is all less than right run's first element
+        var array = Enumerable.Range(1, 25).Concat(Enumerable.Range(100, 25)).ToArray();
+        var sortedArray = Enumerable.Range(1, 25).Concat(Enumerable.Range(100, 25)).ToArray();
+        var sorter = new TimSorter<int>(new TimSorterSettings(), Comparer<int>.Default);
+
+        // Act
+        sorter.Sort(array, Comparer<int>.Default);
+
+        // Assert: Array order will not have changed, and the lenA <= 0 branch should be hit
+        Assert.That(sortedArray, Is.EqualTo(array));
+    }
+
+    [Test]
+    public static void MergeAt_ShouldReturnEarly_WhenLenBIsZero()
+    {
+        // Arrange: right run is all less than left run's last element
+        var array = Enumerable.Range(100, 25).Concat(Enumerable.Range(1, 25)).ToArray();
+        var sortedArray = Enumerable.Range(1, 25).Concat(Enumerable.Range(100, 25)).ToArray();
+        var sorter = new TimSorter<int>(new TimSorterSettings(), Comparer<int>.Default);
+
+        // Act
+        sorter.Sort(array, Comparer<int>.Default);
+
+        // Assert: The left and right sides of the array should have swapped places, and the lenB <= 0 branch should be hit
+        Assert.That(sortedArray, Is.EqualTo(array));
+    }
 }

Algorithms/Sorters/Comparison/TimSorter.cs

@@ -8,7 +8,7 @@ namespace Algorithms.Sorters.Comparison;
 ///
 ///     This class is based on a Java interpretation of Tim Peter's original work.
 ///     Java class is viewable here:
-///     http://cr.openjdk.java.net/~martin/webrevs/openjdk7/timsort/raw_files/new/src/share/classes/java/util/TimSort.java
+///     https://web.archive.org/web/20190119032242/http://cr.openjdk.java.net:80/~martin/webrevs/openjdk7/timsort/raw_files/new/src/share/classes/java/util/TimSort.java
 ///
 ///     Tim Peters's list sort for Python, is described in detail here:
 ///     http://svn.python.org/projects/python/trunk/Objects/listsort.txt
@@ -27,9 +27,6 @@ public class TimSorter<T> : IComparisonSorter<T>
     private readonly int minMerge;
     private readonly int initMinGallop;
 
-    // Pool of reusable TimChunk objects for memory efficiency.
-    private readonly TimChunk<T>[] chunkPool = new TimChunk<T>[2];
-
     private readonly int[] runBase;
     private readonly int[] runLengths;
 
@@ -56,6 +53,11 @@ private class TimChunk<Tc>
     public TimSorter(TimSorterSettings settings, IComparer<T> comparer)
     {
         initMinGallop = minGallop;
+
+        // Using the worst case stack size from the C implementation.
+        // Based on the findings in the original listsort.txt:
+        // ... the stack can never grow larger than about log_base_phi(N) entries, where phi = (1 + sqrt(5)) / 2 ~= 1.618.
+        // Thus a small # of stack slots suffice for very large arrays ...
         runBase = new int[85];
         runLengths = new int[85];
 
@@ -77,7 +79,8 @@ public TimSorter(TimSorterSettings settings, IComparer<T> comparer)
     /// <param name="comparer">Compares elements.</param>
     public void Sort(T[] array, IComparer<T> comparer)
     {
-        this.comparer = comparer;
+        ArgumentNullException.ThrowIfNull(array);
+
         var start = 0;
         var remaining = array.Length;
 

Algorithms/Sorters/Comparison/TimSorterSettings.cs

@@ -1,14 +1,8 @@
 namespace Algorithms.Sorters.Comparison;
 
-public class TimSorterSettings
+public class TimSorterSettings(int minMerge = 32, int minGallop = 7)
 {
-    public int MinMerge { get; }
+    public int MinMerge { get; } = minMerge;
 
-    public int MinGallop { get; }
-
-    public TimSorterSettings(int minMerge = 32, int minGallop = 7)
-    {
-        MinMerge = minMerge;
-        MinGallop = minGallop;
-    }
+    public int MinGallop { get; } = minGallop;
 }

DataStructures.Tests/Hashing/HashTableTests.cs

@@ -113,6 +113,18 @@ public void Add_IncreasesCount_WhenValueAlreadyExists()
         Assert.That(hashTable.Count, Is.EqualTo(2));
     }
 
+    [Test]
+    public void Add_ThrowsException_OnCollision()
+    {
+        // Arrange
+        var hashTable = new HashTable<Collider, int>();
+        hashTable.Add(new Collider(1), 1);
+
+
+        // Act & Assert
+        Assert.Throws<ArgumentException>(() => hashTable.Add(new Collider(1), 2));
+    }
+
     [Test]
     public void Remove_ThrowsException_WhenKeyIsNull()
     {
@@ -154,12 +166,45 @@ public void Remove_DecreasesCount_WhenKeyExists()
     public void Remove_DoesNotDecreaseCount_WhenKeyDoesNotExist()
     {
         var hashTable = new HashTable<string, int>();
-
         hashTable.Remove("a");
 
         Assert.That(hashTable.Count, Is.EqualTo(0));
     }
 
+    [Test]
+    public void Remove_TriggersResizeDown()
+    {
+        var hashTable = new HashTable<int, string>(4);
+        for (var i = 1; i <= 50; i++)
+        {
+            hashTable.Add(i, $"Value{i}");
+        }
+
+        for (var i = 1; i <= 40; i++)
+        {
+            hashTable.Remove(i);
+        }
+
+        Assert.That(hashTable.Capacity, Is.EqualTo(40));
+    }
+
+    [Test]
+    public void Remove_TriggersResizeDown_MinimumOfDefaultCapacity()
+    {
+        var hashTable = new HashTable<int, string>(4);
+        for (var i = 1; i <= 50; i++)
+        {
+            hashTable.Add(i, $"Value{i}");
+        }
+
+        for (var i = 1; i <= 48; i++)
+        {
+            hashTable.Remove(i);
+        }
+
+        Assert.That(hashTable.Capacity, Is.EqualTo(16));
+    }
+
     [Test]
     public void ContainsValue_ReturnsFalse_WhenValueDoesNotExist()
     {
@@ -234,6 +279,17 @@ public void Clear_RemovesAllElements()
         Assert.That(hashTable.ContainsKey("a"), Is.False);
     }
 
+    [Test]
+    public void Clear_ResetsTable()
+    {
+        var hashTable = new HashTable<int, string>();
+        hashTable.Add(1, "A");
+        hashTable.Clear();
+        hashTable.Add(2, "B");
+        Assert.That(hashTable.Count, Is.EqualTo(1));
+        Assert.That(hashTable[2], Is.EqualTo("B"));
+    }
+
     [Test]
     public void Resize_IncreasesCapacity()
     {
@@ -313,6 +369,13 @@ public void Constructor_ThrowsException_WhenLoadFactorIsGreaterThanOne()
         Assert.Throws<ArgumentOutOfRangeException>(() => new HashTable<string, int>(4, 2));
     }
 
+    [Test]
+    public void Constructor_RoundsCapacityToPrime()
+    {
+        var hashTable = new HashTable<int, string>(17);
+        Assert.That(hashTable.Capacity, Is.EqualTo(19));
+    }
+
     [Test]
     public void GetIndex_ThrowsException_WhenKeyIsNull()
     {
@@ -388,6 +451,23 @@ public void Test_NegativeHashKey_ReturnsCorrectValue()
         Assert.That(hashTable[new NegativeHashKey(1)], Is.EqualTo(1));
     }
 
+    [Test]
+    public void Resize_HandlesNegativeHashCodeCorrectly()
+    {
+        // Arrange
+        var hashTable = new HashTable<NegativeHashKey, string>(2);
+
+        // Act
+        hashTable.Add(new NegativeHashKey(1), "A");
+        hashTable.Add(new NegativeHashKey(2), "B");
+        hashTable.Add(new NegativeHashKey(3), "C");
+
+        // Assert
+        Assert.That(hashTable[new NegativeHashKey(1)], Is.EqualTo("A"));
+        Assert.That(hashTable[new NegativeHashKey(2)], Is.EqualTo("B"));
+        Assert.That(hashTable[new NegativeHashKey(3)], Is.EqualTo("C"));
+    }
+
     [Test]
     public void Add_ShouldTriggerResize_WhenThresholdExceeded()
     {
@@ -396,14 +476,14 @@ public void Add_ShouldTriggerResize_WhenThresholdExceeded()
         var hashTable = new HashTable<int, string>(initialCapacity);
 
         // Act
-        for (int i = 1; i <= 4; i++) // Start keys from 1 to avoid default(TKey) = 0 issue
+        for (var i = 1; i <= 32; i++)
         {
             hashTable.Add(i, $"Value{i}");
         }
 
         // Assert
-        hashTable.Capacity.Should().BeGreaterThan(initialCapacity); // Ensure resizing occurred
-        hashTable.Count.Should().Be(4); // Verify count reflects number of added items
+        hashTable.Capacity.Should().BeGreaterThan(initialCapacity);
+        hashTable.Count.Should().Be(32);
     }
 
 
@@ -473,23 +553,28 @@ public void Capacity_Increases_WhenResizeOccurs()
         var initialCapacity = 4;
         var hashTable = new HashTable<int, string>(initialCapacity);
 
-        for (int i = 1; i <= 5; i++)
+        for (var i = 1; i <= 5; i++)
         {
             hashTable.Add(i, $"Value{i}");
         }
 
         hashTable.Capacity.Should().BeGreaterThan(initialCapacity);
     }
-}
-
-public class NegativeHashKey
-{
-    private readonly int id;
 
-    public NegativeHashKey(int id)
+    [Test]
+    public void IndexerSet_Throws_KeyNotFound()
     {
-        this.id = id;
+        // Arrange
+        var hashTable = new HashTable<int, string>();
+
+        // Act & Assert
+        Assert.Throws<KeyNotFoundException>(() => hashTable[1] = "A");
     }
+}
+
+public class NegativeHashKey(int id)
+{
+    private readonly int id = id;
 
     public override int GetHashCode()
     {
@@ -506,3 +591,14 @@ public override bool Equals(object? obj)
         return false;
     }
 }
+
+/// <summary>
+/// Class to simulate hash collisions
+/// </summary>
+/// <param name="id">Id of this object</param>
+public class Collider(int id)
+{
+    private readonly int id = id;
+    public override int GetHashCode() => 42; // Force all instances to collide
+    public override bool Equals(object? obj) => obj is Collider other && other.id == id;
+}