Split updateQuality into smaller chunks and update test according to qualityRange constraint. Update README with steps

Kotlin/README.md

@@ -49,10 +49,19 @@ time is not bloated. Also minimize number of test cases.
 
 ### Test cases generation
 I copied GildedRose class to PlatinumRose and implemented a test which executes both app 
-and then compare results. Time of execution is under 500ms which is great. Also all paths are covered:
+and then compare results. Time of execution is under 500ms which is great. Also, all paths are covered:
 
 ![img_1.png](img_1.png)
 
 One noticeable fact i that items are declared as `var` which makes it possible to update during/after execution.
 In my opinion this is a design issue, code-smell and I'm going to change it to `val`.
 So far I only changed Item to data class so that object comparison is easier, also it seems to be DTO/record so it suppose to be a data class
+
+### updateQuality cleanup
+I cleaned up updateQuality method, split it into 3 separate methods for each case - verified all tests still pass.
+During refactor I also found README file in a main directory in which algorithm was explained - there are extra constraints that are not covered in code:
+- quality of Sulfuras is max 80
+- quality cannot be negative
+
+At this point code is readable but not so extensible because all algos are in a single class which might grow to infinity.
+The usual way to handle this situation is to go with OO approach - I cannot modify Item class so I will implement new one and its specifications.

Kotlin/src/main/kotlin/com/gildedrose/PlatinumRose.kt

@@ -2,57 +2,55 @@ package com.gildedrose
 
 class PlatinumRose(var items: List<Item>) {
 
+    companion object {
+        const val MIN_QUALITY = 0
+        const val REGULAR_ITEM_MAX_QUALITY = 50
+        const val LEGENDARY_ITEM_MAX_QUALITY = 80
+
+        const val AGED_BRIE = "Aged Brie"
+        const val BACKSTAGE_PASSES = "Backstage passes to a TAFKAL80ETC concert"
+        const val LULFURAS_HAND_OF_RAGNAROK = "Sulfuras, Hand of Ragnaros"
+    }
+
     fun updateQuality() {
-        for (i in items.indices) {
+        for (item in items) {
-            if (items[i].name != "Aged Brie" && items[i].name != "Backstage passes to a TAFKAL80ETC concert") {
+            when (item.name) {
-                if (items[i].quality > 0) {
+                AGED_BRIE -> updateQualityForAgedBrie(item)
-                    if (items[i].name != "Sulfuras, Hand of Ragnaros") {
+                BACKSTAGE_PASSES -> updateQualityForBackstagePasses(item)
-                        items[i].quality = items[i].quality - 1
+                LULFURAS_HAND_OF_RAGNAROK -> null
-                    }
+                else -> updateQuality(item)
-                }
-            } else {
-                if (items[i].quality < 50) {
-                    items[i].quality = items[i].quality + 1
-
-                    if (items[i].name == "Backstage passes to a TAFKAL80ETC concert") {
-                        if (items[i].sellIn < 11) {
-                            if (items[i].quality < 50) {
-                                items[i].quality = items[i].quality + 1
-                            }
-                        }
-
-                        if (items[i].sellIn < 6) {
-                            if (items[i].quality < 50) {
-                                items[i].quality = items[i].quality + 1
-                            }
-                        }
-                    }
-                }
-            }
-
-            if (items[i].name != "Sulfuras, Hand of Ragnaros") {
-                items[i].sellIn = items[i].sellIn - 1
-            }
-
-            if (items[i].sellIn < 0) {
-                if (items[i].name != "Aged Brie") {
-                    if (items[i].name != "Backstage passes to a TAFKAL80ETC concert") {
-                        if (items[i].quality > 0) {
-                            if (items[i].name != "Sulfuras, Hand of Ragnaros") {
-                                items[i].quality = items[i].quality - 1
-                            }
-                        }
-                    } else {
-                        items[i].quality = items[i].quality - items[i].quality
-                    }
-                } else {
-                    if (items[i].quality < 50) {
-                        items[i].quality = items[i].quality + 1
-                    }
-                }
             }
+            item.quality = item.quality.coerceIn(0, REGULAR_ITEM_MAX_QUALITY)
         }
     }
 
-}
+    private fun updateQuality(item: Item) {
+        item.quality -= 1
+        item.sellIn -= 1
+        if (item.sellIn < 0) {
+            item.quality -= 1
+        }
+    }
 
+    private fun updateQualityForAgedBrie(item: Item) {
+        item.quality += 1
+        item.sellIn -= 1
+        if (item.sellIn < 0) {
+            item.quality += 1
+        }
+    }
+
+    private fun updateQualityForBackstagePasses(item: Item) {
+        item.quality += 1
+        if (item.sellIn < 11) {
+            item.quality += 1
+        }
+        if (item.sellIn < 6) {
+            item.quality += 1
+        }
+        item.sellIn -= 1
+        if (item.sellIn < 0) {
+            item.quality = 0
+        }
+    }
+}

Kotlin/src/main/kotlin/com/gildedrose/TestcasesGenerator.kt

@@ -1,5 +1,8 @@
 package com.gildedrose
 
+import com.gildedrose.PlatinumRose.Companion.AGED_BRIE
+import com.gildedrose.PlatinumRose.Companion.BACKSTAGE_PASSES
+import com.gildedrose.PlatinumRose.Companion.LULFURAS_HAND_OF_RAGNAROK
 import java.util.*
 
 
@@ -17,9 +20,9 @@ fun generateTestCasesInRanger(names: List<String>, sellInRange: IntRange, qualit
 
 fun main(args: Array<String>) {
     val names = listOf(
-        "Aged Brie",
+        AGED_BRIE,
-        "Backstage passes to a TAFKAL80ETC concert",
+        BACKSTAGE_PASSES,
-        "Sulfuras, Hand of Ragnaros",
+        LULFURAS_HAND_OF_RAGNAROK,
         "new none-existing on code name"
     )
     val sellInRange = -100..100
@@ -32,12 +35,10 @@ fun main(args: Array<String>) {
         val app = GildedRose(listOf(Item(testCase.name, testCase.sellIn, testCase.quality)))
         app.updateQuality()
 
-
         if (testCase.sellIn != app.items[0].sellIn || testCase.quality != app.items[0].quality) {
-            //println("$testCasesCounter: $testCase vs ${app.items[0]}")
+            println("$testCasesCounter: $testCase vs ${app.items[0]}")
             testCasesCounter += 1
         }
     }
     println("Found $testCasesCounter testCasesCounter of out ${allTestCases.size} allTestCases")
-
 }

Kotlin/src/test/kotlin/com/gildedrose/PlatinumRoseTest.kt

@@ -1,5 +1,10 @@
 package com.gildedrose
 
+import com.gildedrose.PlatinumRose.Companion.AGED_BRIE
+import com.gildedrose.PlatinumRose.Companion.BACKSTAGE_PASSES
+import com.gildedrose.PlatinumRose.Companion.LULFURAS_HAND_OF_RAGNAROK
+import com.gildedrose.PlatinumRose.Companion.MIN_QUALITY
+import com.gildedrose.PlatinumRose.Companion.REGULAR_ITEM_MAX_QUALITY
 import org.junit.jupiter.api.Assertions.assertEquals
 import org.junit.jupiter.api.Test
 import java.util.*
@@ -9,13 +14,13 @@ class PlatinumRoseTest {
     @Test
     fun `should update quality for generated test cases and compare with GlidedRose`() {
         val names = listOf(
-            "Aged Brie",
+            AGED_BRIE,
-            "Backstage passes to a TAFKAL80ETC concert",
+            BACKSTAGE_PASSES,
-            "Sulfuras, Hand of Ragnaros",
+            LULFURAS_HAND_OF_RAGNAROK,
             "new none-existing on code name"
         )
         val sellInRange = -100..100
-        val qualityRange = -100..100
+        val qualityRange = MIN_QUALITY..REGULAR_ITEM_MAX_QUALITY
         val allTestCases = generateTestCasesInRanger(names, sellInRange, qualityRange)