Fix aalhour#137 and aalhour#139 issues. Update skipList logic

Author: Gutsonok

DataStructures/Lists/SkipList.cs

@@ -37,9 +37,6 @@ public SkipList()
             _currentMaxLevel = 1;
             _randomizer = new Random();
             _firstNode = new SkipListNode<T>(default(T), MaxLevel);
-
-            for (int i = 0; i < MaxLevel; ++i)
-                _firstNode.Forwards[i] = _firstNode;
         }
 
 
@@ -98,36 +95,43 @@ public void Add(T item)
             var current = _firstNode;
             var toBeUpdated = new SkipListNode<T>[MaxLevel];
 
+            // Get nodes for updated
             for (int i = _currentMaxLevel - 1; i >= 0; --i)
             {
-                while (current.Forwards[i] != _firstNode && current.Forwards[i].Value.IsLessThan(item))
+                while (current.Forwards[i] != null && current.Forwards[i].Value.IsLessThan(item))
+                {
                     current = current.Forwards[i];
+                }
 
                 toBeUpdated[i] = current;
             }
 
-            // Get the next node level, and update list level if required.
-            int lvl = _getNextLevel();
-            if (lvl > _currentMaxLevel)
+            // Desired position to insert key
+            current = current.Forwards[0];
+
+            if(current == null || !current.Value.Equals(item))
             {
-                for (int i = _currentMaxLevel; i < lvl; ++i)
-                    toBeUpdated[i] = _firstNode;
+                // Get the next node level, and update list level if required.
+                int lvl = _getNextLevel();
+                if (lvl > _currentMaxLevel)
+                {
+                    for (int i = _currentMaxLevel; i < lvl; ++i)
+                        toBeUpdated[i] = _firstNode;
 
-                _currentMaxLevel = lvl;
-            }
+                    _currentMaxLevel = lvl;
+                }
 
-            // New node
-            var newNode = new SkipListNode<T>(item, lvl);
+                var newNode = new SkipListNode<T>(item, lvl);
 
-            // Insert the new node into the skip list
-            for (int i = 0; i < lvl; ++i)
-            {
-                newNode.Forwards[i] = toBeUpdated[i].Forwards[i];
-                toBeUpdated[i].Forwards[i] = newNode;
-            }
+                // Insert the new node into the skip list
+                for (int i = 0; i < lvl; ++i)
+                {
+                    newNode.Forwards[i] = toBeUpdated[i].Forwards[i];
+                    toBeUpdated[i].Forwards[i] = newNode;
+                }
 
-            // Increment the count
-            ++_count;
+                ++_count;
+            }
         }
 
         /// <summary>
@@ -151,7 +155,7 @@ public bool Remove(T item, out T deleted)
             // Mark all nodes as toBeUpdated.
             for (int i = _currentMaxLevel - 1; i >= 0; --i)
             {
-                while (current.Forwards[i] != _firstNode && current.Forwards[i].Value.IsLessThan(item))
+                while (current.Forwards[i] != null && current.Forwards[i].Value.IsLessThan(item))
                     current = current.Forwards[i];
 
                 toBeUpdated[i] = current;
@@ -160,7 +164,7 @@ public bool Remove(T item, out T deleted)
             current = current.Forwards[0];
 
             // Return default value of T if the item was not found
-            if (current.Value.IsEqualTo(item) == false)
+            if (current == null || current.Value.IsEqualTo(item) == false)
             {
                 deleted = default(T);
                 return false;
@@ -170,18 +174,20 @@ public bool Remove(T item, out T deleted)
             // Unlink it from the levels where it exists.
             for (int i = 0; i < _currentMaxLevel; ++i)
             {
-                if (toBeUpdated[i].Forwards[i] == current)
+                if (toBeUpdated[i].Forwards[i] != current)
                 {
-                    toBeUpdated[i].Forwards[i] = current.Forwards[i];
+                    break;
                 }
+
+                toBeUpdated[i].Forwards[i] = current.Forwards[i];
             }
 
             // Decrement the count
             --_count;
 
             // Check to see if we've deleted the highest-level node
             // Decrement level
-            while (_currentMaxLevel > 1 && _firstNode.Forwards[_currentMaxLevel - 1] == _firstNode)
+            while (_currentMaxLevel > 1 && _firstNode.Forwards[_currentMaxLevel - 1] == null)
                 --_currentMaxLevel;
 
             // Assign the deleted output parameter to the node.Value
@@ -194,8 +200,7 @@ public bool Remove(T item, out T deleted)
         /// </summary>
         public bool Contains(T item)
         {
-            T itemOut;
-            return Find(item, out itemOut);
+            return Find(item, out var _);
         }
 
         /// <summary>
@@ -207,13 +212,13 @@ public bool Find(T item, out T result)
 
             // Walk after all the nodes that have values less than the node we are looking for
             for (int i = _currentMaxLevel - 1; i >= 0; --i)
-                while (current.Forwards[i] != _firstNode && current.Forwards[i].Value.IsLessThan(item))
+                while (current.Forwards[i] != null && current.Forwards[i].Value.IsLessThan(item))
                     current = current.Forwards[i];
 
             current = current.Forwards[0];
 
             // Return true if we found the element; false otherwise
-            if (current.Value.IsEqualTo(item))
+            if (current != null && current.Value.IsEqualTo(item))
             {
                 result = current.Value;
                 return true;
@@ -290,7 +295,7 @@ public bool TryPeek(out T result)
         public IEnumerator<T> GetEnumerator()
         {
             var node = _firstNode;
-            while (node.Forwards[0] != null && node.Forwards[0] != _firstNode)
+            while (node.Forwards[0] != null && node.Forwards[0] != null)
             {
                 node = node.Forwards[0];
                 yield return node.Value;
@@ -349,9 +354,6 @@ public void Clear()
             _currentMaxLevel = 1;
             _randomizer = new Random();
             _firstNode = new SkipListNode<T>(default(T), MaxLevel);
-
-            for (int i = 0; i < MaxLevel; ++i)
-                _firstNode.Forwards[i] = _firstNode;
         }
         #endregion
 
@@ -364,7 +366,9 @@ private int _getNextLevel()
             int lvl = 1;
 
             while (_randomizer.NextDouble() < Probability && lvl <= _currentMaxLevel && lvl < MaxLevel)
+            {
                 ++lvl;
+            }
 
             return lvl;
         }

UnitTest/DataStructuresTests/SkipListTest.cs

@@ -1,4 +1,4 @@
-using System.Collections.Generic;
+using System.Runtime.InteropServices.ComTypes;
 using DataStructures.Lists;
 using Xunit;
 
@@ -7,51 +7,128 @@ namespace UnitTest.DataStructuresTests
     public static class SkipListTest
     {
         [Fact]
-        public static void AddOneElement()
+        public static void Initialization_ListIsEmpty()
         {
             var skipList = new SkipList<int>();
 
+            Assert.True(skipList.Count == 0);
+            Assert.True(skipList.IsEmpty);
+            Assert.DoesNotContain(0, skipList);
+        }
+
+        [Fact]
+        public static void Add_NullElement_ListContainNullElement()
+        {
+            var skipList = new SkipList<string>();
+
+            skipList.Add(null);
+
+            Assert.True(skipList.Count == 1);
+            Assert.Contains(null, skipList);
+        }
+
+        [Theory]
+        [InlineData(10)]
+        [InlineData(-10)]
+        public static void Add_OneElement_ListContainOneElement(int testValue)
+        {
+            var skipList = new SkipList<int>();
+
+            skipList.Add(testValue);
+
+            Assert.True(skipList.Count == 1);
+            Assert.Contains(testValue, skipList);
+        }
+
+        [Fact]
+        public static void Add_OneElementTwice_ListContainOneElement()
+        {
+            var skipList = new SkipList<int>();
+
+            skipList.Add(10);
             skipList.Add(10);
 
             Assert.True(skipList.Count == 1);
             Assert.Contains(10, skipList);
         }
 
         [Fact]
-        public static void AddBatchOfElements()
+        public static void Add_DefaultElement_ListContainOneElement()
         {
             var skipList = new SkipList<int>();
 
-            for (int i = 100; i > 50; --i)
-            {
-                skipList.Add(i);
-            }
+            skipList.Add(default(int));
 
-            Assert.True(skipList.Count == 50);
-            for (int i = 100; i > 50; --i)
+            Assert.True(skipList.Count == 1);
+            Assert.Contains(0, skipList);
+        }
+
+        [Fact]
+        public static void Add_SomeElements_ListContainTheseElements()
+        {
+            var skipList = new SkipList<int>();
+            skipList.Add(0);
+            skipList.Add(-1);
+            skipList.Add(1);
+            skipList.Add(2);
+
+            Assert.True(skipList.Count == 4);
+            int checkValue = -1;
+            foreach(var item in skipList)
             {
-                Assert.Contains(i, skipList);
+                Assert.Equal(checkValue, item);
+                checkValue++;
             }
         }
 
         [Fact]
-        public static void AddThreeElementsRemoveOneElement()
+        public static void Remove_RemoveOneElement_ListIsEmpty()
         {
             var skipList = new SkipList<int>();
+            skipList.Add(-10);
 
-            skipList.Add(1);
+            skipList.Remove(-10);
+
+            Assert.True(skipList.Count == 0);
+            Assert.DoesNotContain(0, skipList);
+            Assert.DoesNotContain(-10, skipList);
+        }
+
+        [Fact]
+        public static void Remove_RemoveDefaultElement_DoNoRemoveAndListIsEmptyAndAddingIsWork()
+        {
+            var skipList = new SkipList<int>();
+
+            Assert.DoesNotContain(0, skipList);
+            skipList.Remove(default(int));
+
+            Assert.True(skipList.Count == 0);
+            Assert.DoesNotContain(0, skipList);
+            
+            skipList.Add(10);
+            Assert.True(skipList.Count == 1);
+            Assert.Contains(10, skipList);
+        }
+
+        [Fact]
+        public static void Remove_RemoveSomeElements_ListContainOneElement()
+        {
+            var skipList = new SkipList<int>();
+            skipList.Add(0);
+            skipList.Add(-1);
             skipList.Add(2);
             skipList.Add(3);
+
             skipList.Remove(2);
+            skipList.Remove(-1);
+            skipList.Remove(3);
 
-            Assert.True(skipList.Count == 2);
-            Assert.Contains(1, skipList);
-            Assert.Contains(3, skipList);
-            Assert.DoesNotContain(2, skipList);
+            Assert.True(skipList.Count == 1);
+            Assert.Contains(0, skipList);
         }
 
         [Fact]
-        public static void AddAndRemoveBatchOfElements()
+        public static void AddAndRemove_BatchOfElements_ListIsEmpty()
         {
             var skipList = new SkipList<int>();
 
@@ -60,16 +137,22 @@ public static void AddAndRemoveBatchOfElements()
                 skipList.Add(i);
             }
 
+            Assert.True(skipList.Count == 50);
+            for (int i = 100; i > 50; --i)
+            {
+                Assert.Contains(i, skipList);
+            }
+
             for (int i = 100; i > 50; --i)
             {
                 skipList.Remove(i);
             }
 
-            Assert.True(skipList.Count == 0);
             for (int i = 100; i > 50; --i)
             {
                 Assert.DoesNotContain(i, skipList);
             }
+            Assert.True(skipList.Count == 0);
         }
 
         [Fact]