Return isolated windows from Window* methods

This is a squashed merge of PR #655 that closes #652.
morelinq · Nov 20, 2019 · 50b842d · 50b842d
1 parent bf15061
commit 50b842d
Show file tree

Hide file tree

Showing 6 changed files with 154 additions and 12 deletions.
diff --git a/MoreLinq.Test/WindowLeftTest.cs b/MoreLinq.Test/WindowLeftTest.cs
@@ -29,6 +29,51 @@ public void WindowLeftIsLazy()
             new BreakingSequence<int>().WindowLeft(1);
         }
 
+        [Test]
+        public void WindowModifiedBeforeMoveNextDoesNotAffectNextWindow()
+        {
+            var sequence = Enumerable.Range(0, 3);
+            using var e = sequence.WindowLeft(2).GetEnumerator();
+
+            e.MoveNext();
+            var window1 = e.Current;
+            window1[1] = -1;
+            e.MoveNext();
+            var window2 = e.Current;
+
+            Assert.That(window2[0], Is.EqualTo(1));
+        }
+
+        [Test]
+        public void WindowModifiedAfterMoveNextDoesNotAffectNextWindow()
+        {
+            var sequence = Enumerable.Range(0, 3);
+            using var e = sequence.WindowLeft(2).GetEnumerator();
+
+            e.MoveNext();
+            var window1 = e.Current;
+            e.MoveNext();
+            window1[1] = -1;
+            var window2 = e.Current;
+
+            Assert.That(window2[0], Is.EqualTo(1));
+        }
+
+        [Test]
+        public void WindowModifiedDoesNotAffectPreviousWindow()
+        {
+            var sequence = Enumerable.Range(0, 3);
+            using var e = sequence.WindowLeft(2).GetEnumerator();
+
+            e.MoveNext();
+            var window1 = e.Current;
+            e.MoveNext();
+            var window2 = e.Current;
+            window2[0] = -1;
+
+            Assert.That(window1[1], Is.EqualTo(1));
+        }
+
         [Test]
         public void WindowLeftWithNegativeWindowSize()
         {

diff --git a/MoreLinq.Test/WindowRightTest.cs b/MoreLinq.Test/WindowRightTest.cs
@@ -29,6 +29,51 @@ public void WindowRightIsLazy()
             new BreakingSequence<int>().WindowRight(1);
         }
 
+        [Test]
+        public void WindowModifiedBeforeMoveNextDoesNotAffectNextWindow()
+        {
+            var sequence = Enumerable.Range(0, 3);
+            using var e = sequence.WindowRight(2).GetEnumerator();
+
+            e.MoveNext();
+            var window1 = e.Current;
+            window1[0] = -1;
+            e.MoveNext();
+            var window2 = e.Current;
+
+            Assert.That(window2[0], Is.EqualTo(0));
+        }
+
+        [Test]
+        public void WindowModifiedAfterMoveNextDoesNotAffectNextWindow()
+        {
+            var sequence = Enumerable.Range(0, 3);
+            using var e = sequence.WindowRight(2).GetEnumerator();
+
+            e.MoveNext();
+            var window1 = e.Current;
+            e.MoveNext();
+            window1[0] = -1;
+            var window2 = e.Current;
+
+            Assert.That(window2[0], Is.EqualTo(0));
+        }
+
+        [Test]
+        public void WindowModifiedDoesNotAffectPreviousWindow()
+        {
+            var sequence = Enumerable.Range(0, 3);
+            using var e = sequence.WindowRight(2).GetEnumerator();
+
+            e.MoveNext();
+            var window1 = e.Current;
+            e.MoveNext();
+            var window2 = e.Current;
+            window2[0] = -1;
+
+            Assert.That(window1[0], Is.EqualTo(0));
+        }
+
         [Test]
         public void WindowRightWithNegativeWindowSize()
         {

diff --git a/MoreLinq.Test/WindowTest.cs b/MoreLinq.Test/WindowTest.cs
@@ -34,6 +34,51 @@ public void TestWindowIsLazy()
             new BreakingSequence<int>().Window(1);
         }
 
+        [Test]
+        public void WindowModifiedBeforeMoveNextDoesNotAffectNextWindow()
+        {
+            var sequence = Enumerable.Range(0, 3);
+            using var e = sequence.Window(2).GetEnumerator();
+
+            e.MoveNext();
+            var window1 = e.Current;
+            window1[1] = -1;
+            e.MoveNext();
+            var window2 = e.Current;
+
+            Assert.That(window2[0], Is.EqualTo(1));
+        }
+
+        [Test]
+        public void WindowModifiedAfterMoveNextDoesNotAffectNextWindow()
+        {
+            var sequence = Enumerable.Range(0, 3);
+            using var e = sequence.Window(2).GetEnumerator();
+
+            e.MoveNext();
+            var window1 = e.Current;
+            e.MoveNext();
+            window1[1] = -1;
+            var window2 = e.Current;
+
+            Assert.That(window2[0], Is.EqualTo(1));
+        }
+
+        [Test]
+        public void WindowModifiedDoesNotAffectPreviousWindow()
+        {
+            var sequence = Enumerable.Range(0, 3);
+            using var e = sequence.Window(2).GetEnumerator();
+
+            e.MoveNext();
+            var window1 = e.Current;
+            e.MoveNext();
+            var window2 = e.Current;
+            window2[0] = -1;
+
+            Assert.That(window1[1], Is.EqualTo(1));
+        }
+
         /// <summary>
         /// Verify that a negative window size results in an exception
         /// </summary>
@@ -42,7 +87,7 @@ public void TestWindowNegativeWindowSizeException()
         {
             var sequence = Enumerable.Repeat(1, 10);
 
-            AssertThrowsArgument.OutOfRangeException("size",() =>
+            AssertThrowsArgument.OutOfRangeException("size", () =>
                 sequence.Window(-5));
         }
 

diff --git a/MoreLinq/Window.cs b/MoreLinq/Window.cs
@@ -52,21 +52,20 @@ public static IEnumerable<IList<TSource>> Window<TSource>(this IEnumerable<TSour
                 if (i < size)
                     yield break;
 
-                // return the first window (whatever size it may be)
-                yield return window;
-
-                // generate the next window by shifting forward by one item
                 while (iter.MoveNext())
                 {
-                    // NOTE: If we used a circular queue rather than a list,
-                    //       we could make this quite a bit more efficient.
-                    //       Sadly the BCL does not offer such a collection.
+                    // generate the next window by shifting forward by one item
+                    // and do that before exposing the data
                     var newWindow = new TSource[size];
                     Array.Copy(window, 1, newWindow, 0, size - 1);
                     newWindow[size - 1] = iter.Current;
-                    yield return newWindow;
+
+                    yield return window;
                     window = newWindow;
                 }
+
+                // return the last window.
+                yield return window;
             }
         }
 

diff --git a/MoreLinq/WindowLeft.cs b/MoreLinq/WindowLeft.cs
@@ -71,13 +71,18 @@ public static IEnumerable<IList<TSource>> WindowLeft<TSource>(this IEnumerable<T
                     window.Add(item);
                     if (window.Count < size)
                         continue;
+
+                    // prepare next window before exposing data
+                    var nextWindow = new List<TSource>(window.Skip(1));
                     yield return window;
-                    window = new List<TSource>(window.Skip(1));
+                    window = nextWindow;
                 }
                 while (window.Count > 0)
                 {
+                    // prepare next window before exposing data
+                    var nextWindow = new List<TSource>(window.Skip(1));
                     yield return window;
-                    window = new List<TSource>(window.Skip(1));
+                    window = nextWindow;
                 }
             }
         }

diff --git a/MoreLinq/WindowRight.cs b/MoreLinq/WindowRight.cs
@@ -84,8 +84,11 @@ static IEnumerable<IList<TSource>> WindowRightWhile<TSource>(
                 foreach (var item in source)
                 {
                     window.Add(item);
+
+                    // prepare next window before exposing data
+                    var nextWindow = new List<TSource>(predicate(item, window.Count) ? window : window.Skip(1));
                     yield return window;
-                    window = new List<TSource>(predicate(item, window.Count) ? window : window.Skip(1));
+                    window = nextWindow;
                 }
             }
         }