From 2a7257731b7ec96aa602c77ff2e6c61450f79a0d Mon Sep 17 00:00:00 2001 From: Atif Aziz Date: Tue, 21 Nov 2023 21:01:20 +0100 Subject: [PATCH] Refactor "Subsets" core into an iterator method --- MoreLinq.Test/SubsetTest.cs | 24 +++++ MoreLinq/Subsets.cs | 172 +++++++++++------------------------- 2 files changed, 75 insertions(+), 121 deletions(-) diff --git a/MoreLinq.Test/SubsetTest.cs b/MoreLinq.Test/SubsetTest.cs index ef079e166..3d0fb27ce 100644 --- a/MoreLinq.Test/SubsetTest.cs +++ b/MoreLinq.Test/SubsetTest.cs @@ -193,5 +193,29 @@ public void TestKSubsetExpectedResult() foreach (var subset in result) Assert.That(subset, Is.EqualTo(expectedSubsets[index++])); } + + [Test(Description = "https://github.com/morelinq/MoreLINQ/issues/1047")] + public void TestEnumeratorCurrentReturnsSameReferenceOnEachAccess() + { + var source = Seq(1, 2, 3, 4, 5); + + using var e = source.Subsets(3).GetEnumerator(); + var moved = e.MoveNext(); + var first = e.Current; + var second = e.Current; + + Assert.That(moved, Is.True); + Assert.That(first, Is.SameAs(second)); + } + + [Test] + public void TestEachSubsetInstanceIsUnique() + { + var source = Seq(1, 2, 3, 4, 5); + + var subsets = source.Subsets(2).ToArray(); + + Assert.That(subsets[0], Is.Not.SameAs(subsets[1])); + } } } diff --git a/MoreLinq/Subsets.cs b/MoreLinq/Subsets.cs index b5b79cb22..f1167152f 100644 --- a/MoreLinq/Subsets.cs +++ b/MoreLinq/Subsets.cs @@ -18,7 +18,6 @@ namespace MoreLinq { using System; - using System.Collections; using System.Collections.Generic; using System.Linq; @@ -57,23 +56,22 @@ public static IEnumerable> Subsets(this IEnumerable sequence) var sequenceAsList = sequence.ToList(); var sequenceLength = sequenceAsList.Count; - // the first subset is the empty set - yield return new List(); + yield return []; // the first subset is the empty set - // all other subsets are computed using the subset generator - // this check also resolves the case of permuting empty sets - if (sequenceLength > 0) + // next check also resolves the case of permuting empty sets + if (sequenceLength is 0) + yield break; + + // all other subsets are computed using the subset generator... + + for (var k = 1; k < sequenceLength; k++) { - for (var i = 1; i < sequenceLength; i++) - { - // each intermediate subset is a lexographically ordered K-subset - var subsetGenerator = new SubsetGenerator(sequenceAsList, i); - foreach (var subset in subsetGenerator) - yield return subset; - } - - yield return sequenceAsList; // the last subset is the original set itself + // each intermediate subset is a lexographically ordered K-subset + foreach (var subset in Subsets(sequenceAsList, k)) + yield return subset; } + + yield return sequenceAsList; // the last subset is the original set itself } } @@ -115,128 +113,60 @@ public static IEnumerable> Subsets(this IEnumerable sequence, int // preconditions. This however, needs to be carefully considered - and perhaps // may change after further thought and review. - return new SubsetGenerator(sequence, subsetSize); + return _(); IEnumerable> _() + { + foreach (var subset in Subsets(sequence.ToList(), subsetSize)) + yield return subset; + } } - /// - /// This class is responsible for producing the lexographically ordered k-subsets + /// Produces lexographically ordered k-subsets. /// + /// + /// It uses a snapshot of the original sequence, and an iterative, + /// reductive swap algorithm to produce all subsets of a predetermined + /// size less than or equal to the original set size. + /// - sealed class SubsetGenerator : IEnumerable> + static IEnumerable> Subsets(List set, int subsetSize) { - /// - /// SubsetEnumerator uses a snapshot of the original sequence, and an - /// iterative, reductive swap algorithm to produce all subsets of a - /// predetermined size less than or equal to the original set size. - /// - - sealed class SubsetEnumerator : IEnumerator> - { - readonly IList set; // the original set of elements - readonly T[] subset; // the current subset to return - readonly int[] indices; // indices into the original set + // precondition: subsetSize <= set.Count + if (subsetSize > set.Count) + throw new ArgumentOutOfRangeException(nameof(subsetSize), "Subset size must be <= sequence.Count()"); - bool @continue; // termination indicator, set when all subsets have been produced - - int prevSwapIndex; // previous swap index (upper index) - int currSwapIndex; // current swap index (lower index) - int subsetSize; // size of the subset being produced - int setSize; // size of the original set (sequence) - - public SubsetEnumerator(IList set, int subsetSize) - { - // precondition: subsetSize <= set.Count - if (subsetSize > set.Count) - throw new ArgumentOutOfRangeException(nameof(subsetSize), "Subset size must be <= sequence.Count()"); - - // initialize set arrays... - this.set = set; - this.subset = new T[subsetSize]; - this.indices = new int[subsetSize]; - // initialize index counters... - Reset(); - } + var indices = new int[subsetSize]; // indices into the original set + var prevSwapIndex = subsetSize; // previous swap index (upper index) + var currSwapIndex = -1; // current swap index (lower index) + var setSize = set.Count; - public void Reset() + do + { + if (currSwapIndex == -1) { - this.prevSwapIndex = this.subset.Length; - this.currSwapIndex = -1; - this.subsetSize = this.subset.Length; - this.setSize = this.set.Count; - this.@continue = true; + currSwapIndex = 0; + prevSwapIndex = subsetSize; } - - public IList Current => (IList)this.subset.Clone(); - - object IEnumerator.Current => Current; - - public bool MoveNext() + else { - if (!this.@continue) - return false; - - if (this.currSwapIndex == -1) - { - this.currSwapIndex = 0; - this.prevSwapIndex = this.subsetSize; - } - else - { - if (this.currSwapIndex < this.setSize - this.prevSwapIndex) - { - this.prevSwapIndex = 0; - } - this.prevSwapIndex++; - this.currSwapIndex = this.indices[this.subsetSize - this.prevSwapIndex]; - } - - for (var j = 1; j <= this.prevSwapIndex; j++) - this.indices[this.subsetSize + j - this.prevSwapIndex - 1] = this.currSwapIndex + j; - - ExtractSubset(); - - this.@continue = this.indices is [var i, ..] - // .... count of items excluded from the subset - && i != this.setSize - this.subsetSize + 1; - - return true; - } - - void IDisposable.Dispose() { } + if (currSwapIndex < setSize - prevSwapIndex) + prevSwapIndex = 0; - void ExtractSubset() - { - for (var i = 0; i < this.subsetSize; i++) - this.subset[i] = this.set[this.indices[i] - 1]; + prevSwapIndex++; + currSwapIndex = indices[subsetSize - prevSwapIndex]; } - } - readonly IEnumerable sequence; - readonly int subsetSize; + for (var i = 1; i <= prevSwapIndex; i++) + indices[subsetSize + i - prevSwapIndex - 1] = currSwapIndex + i; - public SubsetGenerator(IEnumerable sequence, int subsetSize) - { - if (sequence is null) - throw new ArgumentNullException(nameof(sequence)); - if (subsetSize < 0) - throw new ArgumentOutOfRangeException(nameof(subsetSize), "{subsetSize} must be between 0 and set.Count()"); - this.subsetSize = subsetSize; - this.sequence = sequence; - } + var subset = new T[subsetSize]; // the current subset to return + for (var i = 0; i < subsetSize; i++) + subset[i] = set[indices[i] - 1]; - /// - /// Returns an enumerator that produces all of the k-sized - /// subsets of the initial value set. The enumerator returns - /// and for each subset. - /// - /// an that enumerates all k-sized subsets - - public IEnumerator> GetEnumerator() - { - return new SubsetEnumerator(this.sequence.ToList(), this.subsetSize); + yield return subset; } - - IEnumerator IEnumerable.GetEnumerator() { return GetEnumerator(); } + while (indices is [var fi, ..] + // .... count of items excluded from the subset + && fi != setSize - subsetSize + 1); } } }