From f769be0c95e4ea5beecaa2c2436cb31706e1230f Mon Sep 17 00:00:00 2001 From: Gutsonok Date: Wed, 5 Aug 2020 20:51:49 +0500 Subject: [PATCH 1/5] Fixed Issue #137 Items are not always added to a SkipList --- DataStructures/Lists/SkipList.cs | 47 +++++------ DataStructures/Lists/SkipListNode.cs | 20 +---- UnitTest/DataStructuresTests/SkipListTest.cs | 83 +++++++++++++++++--- 3 files changed, 97 insertions(+), 53 deletions(-) diff --git a/DataStructures/Lists/SkipList.cs b/DataStructures/Lists/SkipList.cs index 57822aca..9567b5c6 100644 --- a/DataStructures/Lists/SkipList.cs +++ b/DataStructures/Lists/SkipList.cs @@ -28,22 +28,6 @@ public class SkipList : ICollection, IEnumerable where T : IComparable< private readonly int MaxLevel = 32; private readonly double Probability = 0.5; - - /// - /// Private helper. Used in Add method. - /// - /// - private int _getNextLevel() - { - int lvl = 0; - - while (_randomizer.NextDouble() < Probability && lvl <= _currentMaxLevel && lvl < MaxLevel) - ++lvl; - - return lvl; - } - - /// /// CONSTRUCTOR /// @@ -94,12 +78,15 @@ public int Level /// /// Access elements by index /// - public T this[int index] + public T this[T item] { get { - // TODO: - throw new NotImplementedException(); + return Find(item, out var result) ? result : throw new KeyNotFoundException(); + } + set + { + Add(item); } } @@ -119,8 +106,6 @@ public void Add(T item) toBeUpdated[i] = current; } - current = current.Forwards[0]; - // Get the next node level, and update list level if required. int lvl = _getNextLevel(); if (lvl > _currentMaxLevel) @@ -150,8 +135,7 @@ public void Add(T item) /// public bool Remove(T item) { - T deleted; - return Remove(item, out deleted); + return Remove(item, out var _); } /// @@ -185,8 +169,12 @@ public bool Remove(T item, out T deleted) // We know that the node is in the list. // Unlink it from the levels where it exists. for (int i = 0; i < _currentMaxLevel; ++i) + { if (toBeUpdated[i].Forwards[i] == current) + { toBeUpdated[i].Forwards[i] = current.Forwards[i]; + } + } // Decrement the count --_count; @@ -367,6 +355,19 @@ public void Clear() } #endregion + /// + /// Private helper. Used in Add method. + /// + /// + private int _getNextLevel() + { + int lvl = 1; + + while (_randomizer.NextDouble() < Probability && lvl <= _currentMaxLevel && lvl < MaxLevel) + ++lvl; + + return lvl; + } } } diff --git a/DataStructures/Lists/SkipListNode.cs b/DataStructures/Lists/SkipListNode.cs index 8188c747..b847b821 100644 --- a/DataStructures/Lists/SkipListNode.cs +++ b/DataStructures/Lists/SkipListNode.cs @@ -4,12 +4,6 @@ namespace DataStructures.Lists { public class SkipListNode : IComparable> where T : IComparable { - /// - /// Instance variables - /// - private T _value; - private SkipListNode[] _forwards; - /// /// CONSTRUCTORS /// @@ -25,20 +19,12 @@ public SkipListNode(T value, int level) /// /// Get and set node's value /// - public virtual T Value - { - get { return this._value; } - private set { this._value = value; } - } + public virtual T Value { get; private set; } /// /// Get and set node's forwards links /// - public virtual SkipListNode[] Forwards - { - get { return this._forwards; } - private set { this._forwards = value; } - } + public virtual SkipListNode[] Forwards { get; private set; } /// /// Return level of node. @@ -55,7 +41,7 @@ public int CompareTo(SkipListNode other) { if (other == null) return -1; - + return this.Value.CompareTo(other.Value); } } diff --git a/UnitTest/DataStructuresTests/SkipListTest.cs b/UnitTest/DataStructuresTests/SkipListTest.cs index 97ab6378..8ec79e29 100644 --- a/UnitTest/DataStructuresTests/SkipListTest.cs +++ b/UnitTest/DataStructuresTests/SkipListTest.cs @@ -1,4 +1,5 @@ -using DataStructures.Lists; +using System.Collections.Generic; +using DataStructures.Lists; using Xunit; namespace UnitTest.DataStructuresTests @@ -6,30 +7,86 @@ namespace UnitTest.DataStructuresTests public static class SkipListTest { [Fact] - public static void DoTest() + public static void AddOneElement() { var skipList = new SkipList(); - for (int i = 100; i >= 50; --i) - skipList.Add(i); + skipList.Add(10); - for (int i = 0; i <= 35; ++i) - skipList.Add(i); + Assert.True(skipList.Count == 1); + Assert.Contains(10, skipList); + } - for (int i = -15; i <= 0; ++i) - skipList.Add(i); + [Fact] + public static void AddBatchOfElements() + { + var skipList = new SkipList(); - for (int i = -15; i >= -35; --i) + for (int i = 100; i > 50; --i) + { skipList.Add(i); + } - Assert.True(skipList.Count == 124); + Assert.True(skipList.Count == 50); + for (int i = 100; i > 50; --i) + { + Assert.Contains(i, skipList); + } + } - skipList.Clear(); + [Fact] + public static void AddThreeElementsRemoveOneElement() + { + var skipList = new SkipList(); + + skipList.Add(1); + skipList.Add(2); + skipList.Add(3); + skipList.Remove(2); + + Assert.True(skipList.Count == 2); + Assert.Contains(1, skipList); + Assert.Contains(3, skipList); + Assert.DoesNotContain(2, skipList); + } - for (int i = 100; i >= 0; --i) + [Fact] + public static void AddAndRemoveBatchOfElements() + { + var skipList = new SkipList(); + + for (int i = 100; i > 50; --i) + { skipList.Add(i); + } + + 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); + } + } + + [Fact] + public static void ClearList() + { + var skipList = new SkipList(); + + skipList.Add(1); + skipList.Add(2); + skipList.Add(3); + skipList.Clear(); - Assert.True(skipList.Count == 101); + Assert.True(skipList.Count == 0); + Assert.True(skipList.IsEmpty); + Assert.DoesNotContain(1, skipList); + Assert.DoesNotContain(2, skipList); + Assert.DoesNotContain(3, skipList); } } } From 9cab15320bf4f2e5e24a4f0396403f8b6468122d Mon Sep 17 00:00:00 2001 From: Gutsonok Date: Wed, 5 Aug 2020 21:07:07 +0500 Subject: [PATCH 2/5] Fixed Issue #139 A new SkipList contains 0 --- DataStructures/Lists/SkipList.cs | 10 +++++++--- UnitTest/DataStructuresTests/SkipListTest.cs | 10 ++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/DataStructures/Lists/SkipList.cs b/DataStructures/Lists/SkipList.cs index 9567b5c6..02822e5b 100644 --- a/DataStructures/Lists/SkipList.cs +++ b/DataStructures/Lists/SkipList.cs @@ -194,8 +194,7 @@ public bool Remove(T item, out T deleted) /// public bool Contains(T item) { - T itemOut; - return Find(item, out itemOut); + return Find(item, out var _); } /// @@ -203,6 +202,12 @@ public bool Contains(T item) /// public bool Find(T item, out T result) { + result = default; + if(IsEmpty) + { + return false; + } + var current = _firstNode; // Walk after all the nodes that have values less than the node we are looking for @@ -219,7 +224,6 @@ public bool Find(T item, out T result) return true; } - result = default(T); return false; } diff --git a/UnitTest/DataStructuresTests/SkipListTest.cs b/UnitTest/DataStructuresTests/SkipListTest.cs index 8ec79e29..7f9f319f 100644 --- a/UnitTest/DataStructuresTests/SkipListTest.cs +++ b/UnitTest/DataStructuresTests/SkipListTest.cs @@ -6,6 +6,16 @@ namespace UnitTest.DataStructuresTests { public static class SkipListTest { + [Fact] + public static void EmptyList() + { + var skipList = new SkipList(); + + Assert.True(skipList.Count == 0); + Assert.True(skipList.IsEmpty); + Assert.DoesNotContain(0, skipList); + } + [Fact] public static void AddOneElement() { From a9ce311547519c982babe26d9771cd9ee8e6dcf1 Mon Sep 17 00:00:00 2001 From: Gutsonok Date: Wed, 5 Aug 2020 21:44:59 +0500 Subject: [PATCH 3/5] Revert "Fixed Issue #139 A new SkipList contains 0" This reverts commit 9cab15320bf4f2e5e24a4f0396403f8b6468122d. --- DataStructures/Lists/SkipList.cs | 10 +++------- UnitTest/DataStructuresTests/SkipListTest.cs | 10 ---------- 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/DataStructures/Lists/SkipList.cs b/DataStructures/Lists/SkipList.cs index 02822e5b..9567b5c6 100644 --- a/DataStructures/Lists/SkipList.cs +++ b/DataStructures/Lists/SkipList.cs @@ -194,7 +194,8 @@ public bool Remove(T item, out T deleted) /// public bool Contains(T item) { - return Find(item, out var _); + T itemOut; + return Find(item, out itemOut); } /// @@ -202,12 +203,6 @@ public bool Contains(T item) /// public bool Find(T item, out T result) { - result = default; - if(IsEmpty) - { - return false; - } - var current = _firstNode; // Walk after all the nodes that have values less than the node we are looking for @@ -224,6 +219,7 @@ public bool Find(T item, out T result) return true; } + result = default(T); return false; } diff --git a/UnitTest/DataStructuresTests/SkipListTest.cs b/UnitTest/DataStructuresTests/SkipListTest.cs index 7f9f319f..8ec79e29 100644 --- a/UnitTest/DataStructuresTests/SkipListTest.cs +++ b/UnitTest/DataStructuresTests/SkipListTest.cs @@ -6,16 +6,6 @@ namespace UnitTest.DataStructuresTests { public static class SkipListTest { - [Fact] - public static void EmptyList() - { - var skipList = new SkipList(); - - Assert.True(skipList.Count == 0); - Assert.True(skipList.IsEmpty); - Assert.DoesNotContain(0, skipList); - } - [Fact] public static void AddOneElement() { From 377a8d1b6e2bf81f318f43e671561c8e4c7d2498 Mon Sep 17 00:00:00 2001 From: Gutsonok Date: Sat, 8 Aug 2020 23:00:29 +0500 Subject: [PATCH 4/5] Fix #138 and #139 issues. Update skipList logic --- DataStructures/Lists/SkipList.cs | 72 +++++------ UnitTest/DataStructuresTests/SkipListTest.cs | 119 ++++++++++++++++--- 2 files changed, 139 insertions(+), 52 deletions(-) diff --git a/DataStructures/Lists/SkipList.cs b/DataStructures/Lists/SkipList.cs index 9567b5c6..4a990403 100644 --- a/DataStructures/Lists/SkipList.cs +++ b/DataStructures/Lists/SkipList.cs @@ -37,9 +37,6 @@ public SkipList() _currentMaxLevel = 1; _randomizer = new Random(); _firstNode = new SkipListNode(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[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(item, lvl); + var newNode = new SkipListNode(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; + } } /// @@ -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,10 +174,12 @@ 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 @@ -181,7 +187,7 @@ public bool Remove(T item, out T deleted) // 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) /// public bool Contains(T item) { - T itemOut; - return Find(item, out itemOut); + return Find(item, out var _); } /// @@ -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 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(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; } diff --git a/UnitTest/DataStructuresTests/SkipListTest.cs b/UnitTest/DataStructuresTests/SkipListTest.cs index 8ec79e29..244700fb 100644 --- a/UnitTest/DataStructuresTests/SkipListTest.cs +++ b/UnitTest/DataStructuresTests/SkipListTest.cs @@ -1,4 +1,4 @@ -using System.Collections.Generic; +using System.Runtime.InteropServices.ComTypes; using DataStructures.Lists; using Xunit; @@ -7,10 +7,45 @@ namespace UnitTest.DataStructuresTests public static class SkipListTest { [Fact] - public static void AddOneElement() + public static void Initialization_ListIsEmpty() { var skipList = new SkipList(); + Assert.True(skipList.Count == 0); + Assert.True(skipList.IsEmpty); + Assert.DoesNotContain(0, skipList); + } + + [Fact] + public static void Add_NullElement_ListContainNullElement() + { + var skipList = new SkipList(); + + 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(); + + skipList.Add(testValue); + + Assert.True(skipList.Count == 1); + Assert.Contains(testValue, skipList); + } + + [Fact] + public static void Add_OneElementTwice_ListContainOneElement() + { + var skipList = new SkipList(); + + skipList.Add(10); skipList.Add(10); Assert.True(skipList.Count == 1); @@ -18,40 +53,82 @@ public static void AddOneElement() } [Fact] - public static void AddBatchOfElements() + public static void Add_DefaultElement_ListContainOneElement() { var skipList = new SkipList(); - 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(); + 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(); + 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(); + + 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(); + 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(); @@ -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] From 321e83881fa840d209c0ef2aa2a1a6c6878b8683 Mon Sep 17 00:00:00 2001 From: Gutsonok Date: Sat, 8 Aug 2020 23:38:05 +0500 Subject: [PATCH 5/5] Fix #140 Program crashes upon search for null in a SkipList --- DataStructures/Lists/SkipList.cs | 20 ++++++++++++++- UnitTest/DataStructuresTests/SkipListTest.cs | 27 ++++++++++++++++++-- 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/DataStructures/Lists/SkipList.cs b/DataStructures/Lists/SkipList.cs index 4a990403..dbb57bc0 100644 --- a/DataStructures/Lists/SkipList.cs +++ b/DataStructures/Lists/SkipList.cs @@ -208,12 +208,31 @@ public bool Contains(T item) /// public bool Find(T item, out T result) { + result = default(T); + var current = _firstNode; + // If find null element then check first element after first node + if (item == null) + { + current = current.Forwards[0]; + return current != null && current.Value == null; + } + + // Skip null element (in first postion) if contain + if (!IsEmpty && current.Forwards[0].Value == null) + { + current = current.Forwards[0]; + } + // 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] != null && current.Forwards[i].Value.IsLessThan(item)) + { current = current.Forwards[i]; + } + } current = current.Forwards[0]; @@ -224,7 +243,6 @@ public bool Find(T item, out T result) return true; } - result = default(T); return false; } diff --git a/UnitTest/DataStructuresTests/SkipListTest.cs b/UnitTest/DataStructuresTests/SkipListTest.cs index 244700fb..cbbe3e15 100644 --- a/UnitTest/DataStructuresTests/SkipListTest.cs +++ b/UnitTest/DataStructuresTests/SkipListTest.cs @@ -1,5 +1,4 @@ -using System.Runtime.InteropServices.ComTypes; -using DataStructures.Lists; +using DataStructures.Lists; using Xunit; namespace UnitTest.DataStructuresTests @@ -16,6 +15,17 @@ public static void Initialization_ListIsEmpty() Assert.DoesNotContain(0, skipList); } + [Fact] + public static void InitializationWithReferencyTypeAsGeneric_ListIsEmpty() + { + var skipList = new SkipList(); + + Assert.True(skipList.Count == 0); + Assert.True(skipList.IsEmpty); + Assert.DoesNotContain(null, skipList); + Assert.DoesNotContain(string.Empty, skipList); + } + [Fact] public static void Add_NullElement_ListContainNullElement() { @@ -27,6 +37,19 @@ public static void Add_NullElement_ListContainNullElement() Assert.Contains(null, skipList); } + [Fact] + public static void Add_NullElementAfterNonNull_ListContainNullElement() + { + var skipList = new SkipList(); + + skipList.Add("1"); + skipList.Add(null); + + Assert.True(skipList.Count == 2); + Assert.Contains(null, skipList); + Assert.Contains("1", skipList); + } + [Theory] [InlineData(10)] [InlineData(-10)]