From 87ca925990490465d7bddfd5db9e85bf5bbfb363 Mon Sep 17 00:00:00 2001 From: Jacek Weremko Date: Sat, 15 Jun 2024 17:06:07 +0200 Subject: [PATCH] Split updateQuality into smaller chunks and update test according to qualityRange constraint. Update README with steps --- Kotlin/README.md | 11 ++- .../kotlin/com/gildedrose/PlatinumRose.kt | 94 +++++++++---------- .../com/gildedrose/TestcasesGenerator.kt | 13 +-- .../kotlin/com/gildedrose/PlatinumRoseTest.kt | 13 ++- 4 files changed, 72 insertions(+), 59 deletions(-) diff --git a/Kotlin/README.md b/Kotlin/README.md index 83f52fac..0b80bad9 100644 --- a/Kotlin/README.md +++ b/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. \ No newline at end of file diff --git a/Kotlin/src/main/kotlin/com/gildedrose/PlatinumRose.kt b/Kotlin/src/main/kotlin/com/gildedrose/PlatinumRose.kt index dc478702..a8e8cd81 100644 --- a/Kotlin/src/main/kotlin/com/gildedrose/PlatinumRose.kt +++ b/Kotlin/src/main/kotlin/com/gildedrose/PlatinumRose.kt @@ -2,57 +2,55 @@ package com.gildedrose class PlatinumRose(var items: List) { + 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) { - if (items[i].name != "Aged Brie" && 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 { - 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 - } - } + for (item in items) { + when (item.name) { + AGED_BRIE -> updateQualityForAgedBrie(item) + BACKSTAGE_PASSES -> updateQualityForBackstagePasses(item) + LULFURAS_HAND_OF_RAGNAROK -> null + else -> updateQuality(item) } + 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 + } + } +} \ No newline at end of file diff --git a/Kotlin/src/main/kotlin/com/gildedrose/TestcasesGenerator.kt b/Kotlin/src/main/kotlin/com/gildedrose/TestcasesGenerator.kt index b259b9ab..b113f6ba 100644 --- a/Kotlin/src/main/kotlin/com/gildedrose/TestcasesGenerator.kt +++ b/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, sellInRange: IntRange, qualit fun main(args: Array) { val names = listOf( - "Aged Brie", - "Backstage passes to a TAFKAL80ETC concert", - "Sulfuras, Hand of Ragnaros", + AGED_BRIE, + BACKSTAGE_PASSES, + LULFURAS_HAND_OF_RAGNAROK, "new none-existing on code name" ) val sellInRange = -100..100 @@ -32,12 +35,10 @@ fun main(args: Array) { 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") - } diff --git a/Kotlin/src/test/kotlin/com/gildedrose/PlatinumRoseTest.kt b/Kotlin/src/test/kotlin/com/gildedrose/PlatinumRoseTest.kt index 290a3262..2d8bea4f 100644 --- a/Kotlin/src/test/kotlin/com/gildedrose/PlatinumRoseTest.kt +++ b/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", - "Backstage passes to a TAFKAL80ETC concert", - "Sulfuras, Hand of Ragnaros", + AGED_BRIE, + BACKSTAGE_PASSES, + 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)