Fix iteration of first sub-generator of CombiningGenerator (#48)

* tests proving the bug * counting invocations to first generator * added hint that name generators can not be expected to be thread safe
Baret · Jun 4, 2024 · f0b2247 · f0b2247
1 parent ba5d614
commit f0b2247
Show file tree

Hide file tree

Showing 3 changed files with 116 additions and 13 deletions.
diff --git a/kotlin-name-generator-api/src/main/kotlin/de/gleex/kng/api/NameGenerator.kt b/kotlin-name-generator-api/src/main/kotlin/de/gleex/kng/api/NameGenerator.kt
@@ -11,6 +11,8 @@ package de.gleex.kng.api
  * ```
  * **Be sure to check for [isAutoResetting] before iterating!** An auto-resetting name
  * generator will create an endless loop.
+ *
+ * **Important hint**: Implementations of this interface DO NOT need to be thread safe!
  */
 interface NameGenerator: Iterator<Name> {
 

diff --git a/kotlin-name-generator/src/main/kotlin/de/gleex/kng/generators/CombiningGenerator.kt b/kotlin-name-generator/src/main/kotlin/de/gleex/kng/generators/CombiningGenerator.kt
@@ -11,6 +11,8 @@ internal class CombiningGenerator(
     private val first: NameGenerator,
     private val second: NameGenerator
 ): NameGenerator {
+    private var firstInvocations = 0
+
     private var secondName: Name? = null
 
     override val isAutoResetting: Boolean by lazy { first.isAutoResetting || second.isAutoResetting }
@@ -21,24 +23,30 @@ internal class CombiningGenerator(
         if(secondName == null) {
             secondName = second.next()
         }
-        if(first.hasNext().not()) {
-            first.reset()
+        if(firstInvocations == first.nameCount) {
+            if(first.isAutoResetting.not()) {
+                first.reset()
+            }
+            firstInvocations = 0
             secondName = second.next()
         }
-        return first.next() + secondName!!
+        val next = first.next()
+        firstInvocations++
+        return next + secondName!!
     }
 
     override fun reset() {
+        firstInvocations = 0
         first.reset()
         second.reset()
         secondName = null
     }
 
     override fun hasNext(): Boolean {
-        return if(second.hasNext().not()) {
-            first.hasNext()
+        return if(firstInvocations == first.nameCount) {
+            first.hasNext() || second.hasNext()
         } else {
-            second.hasNext()
+            first.hasNext()
         }
     }
 }
diff --git a/kotlin-name-generator/src/test/kotlin/de/gleex/kng/generators/CombiningGeneratorTest.kt b/kotlin-name-generator/src/test/kotlin/de/gleex/kng/generators/CombiningGeneratorTest.kt
@@ -15,7 +15,7 @@ import io.kotest.property.exhaustive.boolean
 import io.mockk.*
 
 @OptIn(ExperimentalKotest::class)
-class CombiningGeneratorTest: WordSpec() {
+class CombiningGeneratorTest : WordSpec() {
     init {
         "A combining generator" should {
             val combiningGenerator = CombiningGenerator(
@@ -25,10 +25,12 @@ class CombiningGeneratorTest: WordSpec() {
             afterTest { combiningGenerator.reset() }
 
             "First iterate the first generator, then the second" {
-                listOf("a1", "b1", "c1", "d1",
-                       "a2", "b2", "c2", "d2",
-                       "a3", "b3", "c3", "d3").forAll {
-                           combiningGenerator.nextAsString() shouldBe it
+                listOf(
+                    "a1", "b1", "c1", "d1",
+                    "a2", "b2", "c2", "d2",
+                    "a3", "b3", "c3", "d3"
+                ).forAll {
+                    combiningGenerator.nextAsString() shouldBe it
                 }
             }
 
@@ -94,7 +96,10 @@ class CombiningGeneratorTest: WordSpec() {
                     val secondMock = mockk<NameGenerator> {
                         every { isAutoResetting } returns secondAutoReset
                     }
-                    CombiningGenerator(firstMock, secondMock).isAutoResetting shouldBe (firstAutoReset || secondAutoReset)
+                    CombiningGenerator(
+                        firstMock,
+                        secondMock
+                    ).isAutoResetting shouldBe (firstAutoReset || secondAutoReset)
                 }
             }
 
@@ -139,5 +144,93 @@ class CombiningGeneratorTest: WordSpec() {
                 }
             }
         }
+
+        "Combining CombiningGenerators" should {
+            "iterate over all of them" {
+                val firstGen = nameGenerator(wordListOf("A", "B")) {
+                    autoReset = true
+                }
+                val secondGen = nameGenerator(wordListOf("a", "b")) {
+                    autoReset = true
+                }
+                val thirdGen = nameGenerator(wordListOf("C", "D")) {
+                    autoReset = true
+                }
+                val fourthGen = nameGenerator(wordListOf("c", "d")) {
+                    autoReset = true
+                }
+
+                val firstCombo = CombiningGenerator(firstGen, secondGen)
+                val secondCombo = CombiningGenerator(thirdGen, fourthGen)
+
+                val generatorUnderTest = CombiningGenerator(firstCombo, secondCombo)
+
+                generatorUnderTest.nextAsString() shouldBe "AaCc"
+                generatorUnderTest.nextAsString() shouldBe "BaCc"
+                generatorUnderTest.nextAsString() shouldBe "AbCc"
+                generatorUnderTest.nextAsString() shouldBe "BbCc"
+
+                generatorUnderTest.nextAsString() shouldBe "AaDc"
+                generatorUnderTest.nextAsString() shouldBe "BaDc"
+                generatorUnderTest.nextAsString() shouldBe "AbDc"
+                generatorUnderTest.nextAsString() shouldBe "BbDc"
+
+                generatorUnderTest.nextAsString() shouldBe "AaCd"
+                generatorUnderTest.nextAsString() shouldBe "BaCd"
+                generatorUnderTest.nextAsString() shouldBe "AbCd"
+                generatorUnderTest.nextAsString() shouldBe "BbCd"
+
+                generatorUnderTest.nextAsString() shouldBe "AaDd"
+                generatorUnderTest.nextAsString() shouldBe "BaDd"
+                generatorUnderTest.nextAsString() shouldBe "AbDd"
+                generatorUnderTest.nextAsString() shouldBe "BbDd"
+
+                generatorUnderTest.nextAsString() shouldBe "AaCc"
+            }
+            "work with non-resetting generators" {
+                val firstGen = nameGenerator(wordListOf("A", "B")) {
+                    autoReset = false
+                }
+                val secondGen = nameGenerator(wordListOf("a", "b")) {
+                    autoReset = false
+                }
+                val thirdGen = nameGenerator(wordListOf("C", "D")) {
+                    autoReset = false
+                }
+                val fourthGen = nameGenerator(wordListOf("c", "d")) {
+                    autoReset = false
+                }
+
+                val firstCombo = CombiningGenerator(firstGen, secondGen)
+                val secondCombo = CombiningGenerator(thirdGen, fourthGen)
+
+                val generatorUnderTest = CombiningGenerator(firstCombo, secondCombo)
+
+                generatorUnderTest.nameCount shouldBe 16
+                generatorUnderTest.isAutoResetting shouldBe false
+
+                generatorUnderTest.nextAsString() shouldBe "AaCc"
+                generatorUnderTest.nextAsString() shouldBe "BaCc"
+                generatorUnderTest.nextAsString() shouldBe "AbCc"
+                generatorUnderTest.nextAsString() shouldBe "BbCc"
+
+                generatorUnderTest.nextAsString() shouldBe "AaDc"
+                generatorUnderTest.nextAsString() shouldBe "BaDc"
+                generatorUnderTest.nextAsString() shouldBe "AbDc"
+                generatorUnderTest.nextAsString() shouldBe "BbDc"
+
+                generatorUnderTest.nextAsString() shouldBe "AaCd"
+                generatorUnderTest.nextAsString() shouldBe "BaCd"
+                generatorUnderTest.nextAsString() shouldBe "AbCd"
+                generatorUnderTest.nextAsString() shouldBe "BbCd"
+
+                generatorUnderTest.nextAsString() shouldBe "AaDd"
+                generatorUnderTest.nextAsString() shouldBe "BaDd"
+                generatorUnderTest.nextAsString() shouldBe "AbDd"
+                generatorUnderTest.nextAsString() shouldBe "BbDd"
+
+                shouldThrow<NameGeneratorExhaustedException> { generatorUnderTest.next() }
+            }
+        }
     }
-}
+}