Return isolated windows from Window* methods

[Orace]

This allow tests from #653 to pass and fix #652

[Orace]

@atifaziz you can review this before #656
Fix #652
Fix #654

[atifaziz]

Thanks for the fix. The only trouble with the correction is that it now eagerly allocates and prepares the next window, which is wasteful if the iteration never resumes. I like how it's done in F#:

// Copyright (c) Microsoft Corporation.  All Rights Reserved.  See License.txt in the project root for license information.

// windowed : int -> seq<'T> -> seq<'T[]>
[<CompiledName("Windowed")>]
let windowed windowSize (source: seq<_>) =
	checkNonNull "source" source
	if windowSize <= 0 then invalidArgFmt "windowSize" "{0}\nwindowSize = {1}"
								[|SR.GetString SR.inputMustBePositive; windowSize|]
	seq {
		let arr = Array.zeroCreateUnchecked windowSize
		let r = ref (windowSize - 1)
		let i = ref 0
		use e = source.GetEnumerator()
		while e.MoveNext() do
			arr.[!i] <- e.Current
			i := (!i + 1) % windowSize
			if !r = 0 then
				if windowSize < 32 then
					yield Array.init windowSize (fun j -> arr.[(!i+j) % windowSize])
				else
					let result = Array.zeroCreateUnchecked windowSize
					Array.Copy(arr, !i, result, 0, windowSize - !i)
					Array.Copy(arr, 0, result, windowSize - !i, !i)
					yield result
			else r := (!r - 1)
	}

A single working and circular window is maintained internally and it's re-aligned and copied to a new window when it's time to yield. Can we take inspiration from that?

[Orace]

The only trouble with the new correction is that it now eagerly allocates and prepares the next window

this should be fixed in #656 where the only case an array is allocated and not yield is for a call to Window with a sequence shorter than the Window size.

@atifaziz I do a re-read of the code

With current implementation of Window (and of WindowImplem in #656), at each iteration:

• a new array of size N is allocated.
• N-1 data are copied from an array to an other

And when the sequence reach its end, all allocated array have been passed to the caller

With your proposed implementation (has I understand it), at each iteration:

• a new array of size N is allocated.
• k data are copied from an array to an other
• N-k data are copied from an array to an other

And when the sequence reach its end, one array has not been passed to the caller

In any case a new array of size N is (and have to, since we give the data to the user) allocated.
In any case (but the last yield), we have to keep the data to be able to build the next Window.

The only "we can do it later" I see is:

yield return window;
newWindow[size - 1] = iter.Current;

instead of

newWindow[size - 1] = iter.Current;
yield return window;

I will put it in #656 ;)
I'm for the current implementation

[atifaziz]

And when the sequence reach its end, all allocated array have been passed to the caller

How do you know you'll reach the end of the sequence. The caller is in control of asking the next window and may stop iteration earlier due to some condition.

[atifaziz]

Small nitpicks for now.

[Orace]

And when the sequence reach its end, all allocated array have been passed to the caller

How do you know you'll reach the end of the sequence. The caller is in control of asking the next window and may stop iteration earlier due to some condition.

It's an hypothesis.
For a partially enumerated sequence both algorithm instantiate the same number of array (of the size of the window). The number of instantiated array is number of yielded window + 1 in both implementations.
For a fully enumerated sequence your algorithm instantiate one array more than the actual code.

[atifaziz]

Thank you for this fix and the tests! 👍

[atifaziz]

@atifaziz I do a re-read of the code

@Orace I re-read as well, and in detail this time, and it looks good! 💯