Sonar fixes

domain/src/commonMain/kotlin/ch/dissem/yaep/domain/GameCell.kt

@@ -19,7 +19,7 @@ class GameCell<C : ItemClass<C>>(
                 selectionChangedListeners.forEach { listener -> listener(value) }
             }
         }
-    val options: ObservableSet<Item<C>> = ObservableSet(options) { before, after ->
+    val options: ObservableSet<Item<C>> = ObservableSet(options) { _, after ->
         optionsChangedListeners.forEach { listener ->
             listener(after)
         }

domain/src/commonMain/kotlin/ch/dissem/yaep/domain/GameRow.kt

@@ -37,7 +37,7 @@ class GameRow<C : ItemClass<C>>(
                         it.options.clear()
                         it.options.add(it.selection!!)
                     }
-                cellsWithSelection.mapNotNull { it.selection }.toMutableSet()
+                cellsWithSelection.mapNotNull { it.selection }.toSet()
             }
             filter { it.selection == null }
                 .forEach { it.options.removeAll(selections) }

domain/src/commonMain/kotlin/ch/dissem/yaep/domain/clues.kt

@@ -75,10 +75,12 @@ class NeighbourClue<A : ItemClass<A>, B : ItemClass<B>>(val a: Item<A>, val b: I
 
         for (iX in rowX.indices) {
             val cellX = rowX[iX]
-            if (cellX.mayBe(x, mayHaveSelection = false)) {
-                if (!rowY.getOrNull(iX - 1).mayBe(y) && !rowY.getOrNull(iX + 1).mayBe(y)) {
-                    removed = cellX.options.remove(x) || removed
-                }
+            if (
+                cellX.mayBe(x, mayHaveSelection = false)
+                && !rowY.getOrNull(iX - 1).mayBe(y)
+                && !rowY.getOrNull(iX + 1).mayBe(y)
+            ) {
+                removed = cellX.options.remove(x) || removed
             }
         }
 
@@ -185,51 +187,71 @@ class TripletClue<A : ItemClass<A>, B : ItemClass<B>, C : ItemClass<C>>(
         val ic by lazy { rowC.indexOf(cType) }
 
         if (ia != -1) {
-            return when (ib) {
-                -1 -> when (ic) {
-                    -1 -> (rowB.getOrNull(ia - 1).hasNoSelection() && rowC.getOrNull(ia - 2)
-                        .hasNoSelection()) ||
-                            (rowB.getOrNull(ia + 1).hasNoSelection() && rowC.getOrNull(ia + 2)
-                                .hasNoSelection())
-
-                    ia - 2 -> rowB.getOrNull(ia - 1).hasNoSelection()
-                    ia + 2 -> rowB.getOrNull(ia + 1).hasNoSelection()
-                    else -> false
-                }
-
-                ia - 1 -> when (ic) {
-                    -1 -> rowC.getOrNull(ia - 2).hasNoSelection()
-                    ia - 2 -> true
-                    else -> false
-                }
-
-                ia + 1 -> when (ic) {
-                    -1 -> rowC.getOrNull(ia + 2).hasNoSelection()
-                    ia + 2 -> true
-                    else -> false
-                }
-
-                else -> false
-            }
+            return isValidWithASet(ia, ib, ic, rowB, rowC)
         }
         if (ib != -1) {
-            when (ic) {
-                -1 -> return (rowA.getOrNull(ib - 1).hasNoSelection() && rowC.getOrNull(ib + 1)
-                    .hasNoSelection()) ||
-                        (rowA.getOrNull(ib + 1).hasNoSelection() && rowC.getOrNull(ib - 1)
-                            .hasNoSelection())
-
-                ib - 1 -> return rowA.getOrNull(ib + 1).hasNoSelection()
-                ib + 1 -> return rowA.getOrNull(ib - 1).hasNoSelection()
-            }
+            return isValidWithBSet(ib, ic, rowA, rowC)
         }
         if (ic != -1) {
-            return (rowB.getOrNull(ic - 1).hasNoSelection() && rowA.getOrNull(ic - 2)
-                .hasNoSelection()) ||
-                    (rowB.getOrNull(ic + 1).hasNoSelection() && rowA.getOrNull(ic + 2)
-                        .hasNoSelection())
+            return isValidWithCSet(ic, rowA, rowB)
         }
-        return rowA.mapIndexed { index, gameCell -> if (gameCell.hasNoSelection()) index else null }
+        return isValidWithNoneSet(rowA, rowB, rowC)
+    }
+
+    private fun isValidWithASet(
+        ia: Int,
+        ib: Int,
+        ic: Int,
+        rowB: GameRow<B>,
+        rowC: GameRow<C>
+    ): Boolean =
+        when (ib) {
+            -1 -> when (ic) {
+                -1 -> (rowB.getOrNull(ia - 1).hasNoSelection() && rowC.getOrNull(ia - 2)
+                    .hasNoSelection()) ||
+                        (rowB.getOrNull(ia + 1).hasNoSelection() && rowC.getOrNull(ia + 2)
+                            .hasNoSelection())
+
+                ia - 2 -> rowB.getOrNull(ia - 1).hasNoSelection()
+                ia + 2 -> rowB.getOrNull(ia + 1).hasNoSelection()
+                else -> false
+            }
+
+            ia - 1 -> when (ic) {
+                -1 -> rowC.getOrNull(ia - 2).hasNoSelection()
+                ia - 2 -> true
+                else -> false
+            }
+
+            ia + 1 -> when (ic) {
+                -1 -> rowC.getOrNull(ia + 2).hasNoSelection()
+                ia + 2 -> true
+                else -> false
+            }
+
+            else -> false
+        }
+
+    private fun isValidWithBSet(ib: Int, ic: Int, rowA: GameRow<A>, rowC: GameRow<C>): Boolean =
+        when (ic) {
+            -1 -> (rowA.getOrNull(ib - 1).hasNoSelection() && rowC.getOrNull(ib + 1)
+                .hasNoSelection()) ||
+                    (rowA.getOrNull(ib + 1).hasNoSelection() && rowC.getOrNull(ib - 1)
+                        .hasNoSelection())
+
+            ib - 1 -> rowA.getOrNull(ib + 1).hasNoSelection()
+            ib + 1 -> rowA.getOrNull(ib - 1).hasNoSelection()
+            else -> false
+        }
+
+    private fun isValidWithCSet(ic: Int, rowA: GameRow<A>, rowB: GameRow<B>): Boolean =
+        (rowB.getOrNull(ic - 1).hasNoSelection() && rowA.getOrNull(ic - 2)
+            .hasNoSelection()) ||
+                (rowB.getOrNull(ic + 1).hasNoSelection() && rowA.getOrNull(ic + 2)
+                    .hasNoSelection())
+
+    private fun isValidWithNoneSet(rowA: GameRow<A>, rowB: GameRow<B>, rowC: GameRow<C>): Boolean =
+        rowA.mapIndexed { index, gameCell -> if (gameCell.hasNoSelection()) index else null }
             .filterNotNull()
             .any { index ->
                 (rowB.getOrNull(index - 1).hasNoSelection() && rowC.getOrNull(index - 2)
@@ -237,7 +259,6 @@ class TripletClue<A : ItemClass<A>, B : ItemClass<B>, C : ItemClass<C>>(
                         (rowB.getOrNull(index + 1).hasNoSelection() && rowC.getOrNull(index + 2)
                             .hasNoSelection())
             }
-    }
 
     override fun removeForbiddenOptions(grid: Grid): Boolean {
         val rowA = grid[aType.companion]
@@ -247,49 +268,62 @@ class TripletClue<A : ItemClass<A>, B : ItemClass<B>, C : ItemClass<C>>(
         var removed = false
 
         for (i in rowA.indices) {
-            val cellA = rowA[i]
-            if (cellA.mayBe(a, mayHaveSelection = false)) {
-                val cellBR = rowB.getOrNull(i + 1)
-                val cellCR = rowC.getOrNull(i + 2)
-                val cellBL = rowB.getOrNull(i - 1)
-                val cellCL = rowC.getOrNull(i - 2)
-
-                if (!(cellBR.mayBe(b) && cellCR.mayBe(c)) && !(cellBL.mayBe(b) && cellCL.mayBe(c))) {
-                    removed = cellA.options.remove(a) || removed
-                }
-            }
+            removed = removeForbiddenOptionsGivenX(
+                i = i,
+                x = Group(a, rowA),
+                y = Group(b, rowB, 1),
+                z = Group(c, rowC, 2)
+            ) || removed
         }
         for (i in rowB.indices) {
-            val cellB = rowB[i]
-            if (cellB.mayBe(b, mayHaveSelection = false)) {
-                val cellAL = rowA.getOrNull(i - 1)
-                val cellAR = rowA.getOrNull(i + 1)
-                val cellCL = rowC.getOrNull(i - 1)
-                val cellCR = rowC.getOrNull(i + 1)
-
-                if (!(cellAL.mayBe(a) && cellCR.mayBe(c)) && !(cellCL.mayBe(c) && cellAR.mayBe(a))) {
-                    removed = cellB.options.remove(b) || removed
-                }
-            }
+            removed = removeForbiddenOptionsGivenX(
+                i = i,
+                x = Group(b, rowB),
+                y = Group(a, rowA, 1),
+                z = Group(c, rowC, -1)
+            ) || removed
         }
 
         for (i in rowC.indices) {
-            val cellC = rowC[i]
-            if (cellC.mayBe(c, mayHaveSelection = false)) {
-                val cellBR = rowB.getOrNull(i + 1)
-                val cellAR = rowA.getOrNull(i + 2)
-                val cellBL = rowB.getOrNull(i - 1)
-                val cellAL = rowA.getOrNull(i - 2)
-
-                if (!(cellBR.mayBe(b) && cellAR.mayBe(a)) && !(cellBL.mayBe(b) && cellAL.mayBe(a))) {
-                    removed = cellC.options.remove(c) || removed
-                }
-            }
+            removed = removeForbiddenOptionsGivenX(
+                i = i,
+                x = Group(c, rowC),
+                y = Group(b, rowB, 1),
+                z = Group(a, rowA, 2)
+            ) || removed
         }
-
         return removed
     }
 
+    private fun <X : ItemClass<X>, Y : ItemClass<Y>, Z : ItemClass<Z>> removeForbiddenOptionsGivenX(
+        i: Int,
+        x: Group<X>,
+        y: Group<Y>,
+        z: Group<Z>
+    ): Boolean {
+        val cellX = x.row[i]
+        if (cellX.mayBe(x.item, mayHaveSelection = false)) {
+            val cellYR = y.row.getOrNull(i + y.offset)
+            val cellZR = z.row.getOrNull(i + z.offset)
+            val cellYL = y.row.getOrNull(i - y.offset)
+            val cellZL = z.row.getOrNull(i - z.offset)
+
+            if (
+                !(cellYR.mayBe(y.item) && cellZR.mayBe(z.item))
+                && !(cellYL.mayBe(y.item) && cellZL.mayBe(z.item))
+            ) {
+                return cellX.options.remove(x.item)
+            }
+        }
+        return false
+    }
+
+    private class Group<T : ItemClass<T>>(
+        val item: Item<T>,
+        val row: GameRow<T>,
+        val offset: Int = 0
+    )
+
     override fun toString(): String =
         "$bType is between the neighbours $aType and $cType to both sides"
 
@@ -343,15 +377,11 @@ class SameColumnClue<A : ItemClass<A>, B : ItemClass<B>>(val a: Item<A>, val b:
             val cellA = rowA[i]
             val cellB = rowB[i]
 
-            if (cellB.hasNoSelection()) {
-                if (!cellA.mayBe(a)) {
-                    removed = cellB.options.remove(b) || removed
-                }
+            if (cellB.hasNoSelection() && !cellA.mayBe(a)) {
+                removed = cellB.options.remove(b) || removed
             }
-            if (cellA.hasNoSelection()) {
-                if (!cellB.mayBe(b)) {
-                    removed = cellA.options.remove(a) || removed
-                }
+            if (cellA.hasNoSelection() && !cellB.mayBe(b)) {
+                removed = cellA.options.remove(a) || removed
             }
         }
         return removed

domain/src/commonTest/kotlin/ch/dissem/yaep/domain/GameSolverTest.kt

@@ -14,7 +14,7 @@ class GameSolverTest {
     fun `ensure there are no unnecessary clues`() {
         var game: Game
         var neighbours: List<NeighbourClue<*, *>>
-        repeat(100) {
+        repeat(10) {
             game = generateGame()
             val triplets = game.horizontalClues.filterIsInstance<TripletClue<*, *, *>>()
             neighbours = game.horizontalClues.filterIsInstance<NeighbourClue<*, *>>()

domain/src/commonTest/kotlin/ch/dissem/yaep/domain/GameTest.kt

@@ -19,7 +19,7 @@ class GameTest {
 
     @Test
     fun `ensure generated games are solvable`() {
-        val tries = 1000
+        val tries = 100
         var fastest = 500.milliseconds
         var slowest = 0.milliseconds
         var total = 0.milliseconds