From fc415450068461163e560ebbfd561d04a13098eb Mon Sep 17 00:00:00 2001
From: benbierens <thatbenbierens@gmail.com>
Date: Tue, 5 Dec 2023 10:06:47 +0100
Subject: [PATCH] debugs indexing strategies

---
 codex/manifest/indexingstrategy.nim  | 23 ++++++++++---------
 tests/codex/testindexingstrategy.nim | 33 ++++++++++++++++++++++------
 2 files changed, 39 insertions(+), 17 deletions(-)

diff --git a/codex/manifest/indexingstrategy.nim b/codex/manifest/indexingstrategy.nim
index 19f8d9aa..2faa2f83 100644
--- a/codex/manifest/indexingstrategy.nim
+++ b/codex/manifest/indexingstrategy.nim
@@ -1,6 +1,10 @@
 import std/sequtils
 import ../utils
 
+# I'm choosing to use an assert here because:
+# 1. These are a programmer errors and *should not* happen during application runtime.
+# 2. Users don't have to deal with Result types.
+
 type
   # Representing a strategy for grouping indices (of blocks usually)
   # Given an interation-count as input, will produce a seq of
@@ -25,15 +29,17 @@ type
 
 proc assertIteration(self: IndexingStrategy, iteration: int): void =
   if iteration >= self.numberOfIterations:
-    # I'm choosing to use an assert here because:
-    # 1. This is a programmer error and *should not* happen during application runtime.
-    # 2. Users don't have to deal with Result types.
     raiseAssert("Indexing iteration can't be greater than or equal to numberOfIterations.")
 
 method getIndicies*(self: IndexingStrategy, iteration: int): seq[int] {.base.} =
   raiseAssert("Not implemented")
 
 proc new*(T: type IndexingStrategy, firstIndex, lastIndex, numberOfIterations: int): T =
+  if firstIndex > lastIndex:
+    raiseAssert("firstIndex (" & $firstIndex & ") can't be greater than lastIndex (" & $lastIndex & ")")
+  if numberOfIterations <= 0:
+    raiseAssert("numberOfIteration (" & $numberOfIterations & ") must be greater than zero.")
+
   T(
     firstIndex: firstIndex,
     lastIndex: lastIndex,
@@ -45,14 +51,11 @@ method getIndicies*(self: LinearIndexingStrategy, iteration: int): seq[int] =
   self.assertIteration(iteration)
 
   let
-    first = self.firstIndex + iteration * self.step
+    first = self.firstIndex + iteration * (self.step + 1)
     last = min(first + self.step, self.lastIndex)
-  toSeq(countup(first, last - 1, 1))
+
+  toSeq(countup(first, last, 1))
 
 method getIndicies*(self: SteppedIndexingStrategy, iteration: int): seq[int] =
   self.assertIteration(iteration)
-
-  let
-    first = self.firstIndex + iteration
-    last = max(first, first - 1 + (self.step * self.numberOfIterations))
-  toSeq(countup(first, last, self.numberOfIterations))
+  toSeq(countup(self.firstIndex + iteration, self.lastIndex, self.numberOfIterations))
diff --git a/tests/codex/testindexingstrategy.nim b/tests/codex/testindexingstrategy.nim
index c2fe212d..ffce009f 100644
--- a/tests/codex/testindexingstrategy.nim
+++ b/tests/codex/testindexingstrategy.nim
@@ -6,7 +6,7 @@ import ./helpers
 
 import codex/manifest/indexingstrategy
 
-for offset in @[0, 1, 100]:
+for offset in @[0, 1, 2, 100]:
   checksuite "Indexing strategies (Offset: " & $offset & ")":
     let
       firstIndex = 0 + offset
@@ -17,13 +17,13 @@ for offset in @[0, 1, 100]:
 
     test "linear":
       check:
-        linear.getIndicies(0) == @[0, 1, 2, 3].mapIt(it + offset)
-        linear.getIndicies(1) == @[4, 5, 6, 7].mapIt(it + offset)
-        linear.getIndicies(2) == @[8, 9, 10, 11].mapIt(it + offset)
+        linear.getIndicies(0) == @[0, 1, 2, 3, 4].mapIt(it + offset)
+        linear.getIndicies(1) == @[5, 6, 7, 8, 9].mapIt(it + offset)
+        linear.getIndicies(2) == @[10, 11, 12].mapIt(it + offset)
 
     test "stepped":
       check:
-        stepped.getIndicies(0) == @[0, 3, 6, 9].mapIt(it + offset)
+        stepped.getIndicies(0) == @[0, 3, 6, 9, 12].mapIt(it + offset)
         stepped.getIndicies(1) == @[1, 4, 7, 10].mapIt(it + offset)
         stepped.getIndicies(2) == @[2, 5, 8, 11].mapIt(it + offset)
 
@@ -32,11 +32,30 @@ checksuite "Indexing strategies":
     linear = LinearIndexingStrategy.new(0, 10, 3)
     stepped = SteppedIndexingStrategy.new(0, 10, 3)
 
-  test "smallest range":
-    let s = SteppedIndexingStrategy.new(0, 0, 1)
+  test "smallest range 0":
+    let
+      l = LinearIndexingStrategy.new(0, 0, 1)
+      s = SteppedIndexingStrategy.new(0, 0, 1)
     check:
+      l.getIndicies(0) == @[0]
       s.getIndicies(0) == @[0]
 
+  test "smallest range 1":
+    let
+      l = LinearIndexingStrategy.new(0, 1, 1)
+      s = SteppedIndexingStrategy.new(0, 1, 1)
+    check:
+      l.getIndicies(0) == @[0, 1]
+      s.getIndicies(0) == @[0, 1]
+
+  test "first index must be smaller than last index":
+    expect AssertionDefect:
+      discard LinearIndexingStrategy.new(10, 0, 1)
+
+  test "numberOfIterations must be greater than zero":
+    expect AssertionDefect:
+      discard LinearIndexingStrategy.new(0, 10, 0)
+
   test "linear - oob":
     expect AssertionDefect:
       discard linear.getIndicies(3)