From bb2257fced19bab60d384615e408f688ed3a3a23 Mon Sep 17 00:00:00 2001 From: Christopher Siewert Date: Sat, 12 Feb 2022 00:15:40 -0800 Subject: [PATCH] Java refactor --- Java/chris-readme.txt | 7 + .../com/gildedrose/BackstagePassesHolder.java | 24 ++++ .../main/java/com/gildedrose/BrieHolder.java | 17 +++ .../com/gildedrose/GenericItemHolder.java | 20 +++ .../main/java/com/gildedrose/GildedRose.java | 56 +------- .../main/java/com/gildedrose/ItemHolder.java | 17 +++ .../com/gildedrose/ItemHolderFactory.java | 15 +++ .../java/com/gildedrose/SulfurasHolder.java | 11 ++ .../java/com/gildedrose/GildedRoseTest.java | 122 +++++++++++++++++- .../com/gildedrose/ItemHolderFactoryTest.java | 120 +++++++++++++++++ 10 files changed, 358 insertions(+), 51 deletions(-) create mode 100644 Java/chris-readme.txt create mode 100644 Java/src/main/java/com/gildedrose/BackstagePassesHolder.java create mode 100644 Java/src/main/java/com/gildedrose/BrieHolder.java create mode 100644 Java/src/main/java/com/gildedrose/GenericItemHolder.java create mode 100644 Java/src/main/java/com/gildedrose/ItemHolder.java create mode 100644 Java/src/main/java/com/gildedrose/ItemHolderFactory.java create mode 100644 Java/src/main/java/com/gildedrose/SulfurasHolder.java create mode 100644 Java/src/test/java/com/gildedrose/ItemHolderFactoryTest.java diff --git a/Java/chris-readme.txt b/Java/chris-readme.txt new file mode 100644 index 00000000..e80ef107 --- /dev/null +++ b/Java/chris-readme.txt @@ -0,0 +1,7 @@ +ItemHolder is an abstract class that implements the update template (first update quality then sellIn). +Concrete classes implement algorithm for each item type. +ItemHolder object creation is put into its own factory class as it will change with new item types. + +Improvements: +Test code is highly duplicated. I assume Junit has paramaterized tests, just didn't bother looking it up. +ItemHolders do checks for max and min values. Maybe those belong in setSellIn/setQuality methods? diff --git a/Java/src/main/java/com/gildedrose/BackstagePassesHolder.java b/Java/src/main/java/com/gildedrose/BackstagePassesHolder.java new file mode 100644 index 00000000..cb0c6f4b --- /dev/null +++ b/Java/src/main/java/com/gildedrose/BackstagePassesHolder.java @@ -0,0 +1,24 @@ +package com.gildedrose; + +public class BackstagePassesHolder extends ItemHolder { + + public BackstagePassesHolder(Item item) { + super(item); + } + public void updateQuality() { + if (this.item.sellIn > 10) { + this.item.quality += 1; + } else if (this.item.sellIn > 5) { + this.item.quality += 2; + } else if (this.item.sellIn > 0) { + this.item.quality += 3; + } else if (this.item.sellIn <= 0) { + this.item.quality = 0; + } + this.item.quality = Math.min(this.item.quality, 50); + } + + public void updateSellIn() { + this.item.sellIn -= 1; + } +} diff --git a/Java/src/main/java/com/gildedrose/BrieHolder.java b/Java/src/main/java/com/gildedrose/BrieHolder.java new file mode 100644 index 00000000..74853fe0 --- /dev/null +++ b/Java/src/main/java/com/gildedrose/BrieHolder.java @@ -0,0 +1,17 @@ +package com.gildedrose; + +public class BrieHolder extends ItemHolder { + + public BrieHolder(Item item) { + super(item); + } + public void updateQuality() { + if (this.item.quality < 50) { + this.item.quality += 1; + } + } + + public void updateSellIn() { + this.item.sellIn -= 1; + } +} diff --git a/Java/src/main/java/com/gildedrose/GenericItemHolder.java b/Java/src/main/java/com/gildedrose/GenericItemHolder.java new file mode 100644 index 00000000..7ebd915b --- /dev/null +++ b/Java/src/main/java/com/gildedrose/GenericItemHolder.java @@ -0,0 +1,20 @@ +package com.gildedrose; + +public class GenericItemHolder extends ItemHolder { + + public GenericItemHolder(Item item) { + super(item); + } + public void updateQuality() { + if (this.item.sellIn > 0) { + this.item.quality -= 1; + } else { + this.item.quality -= 2; + } + this.item.quality = Math.max(this.item.quality, 0); + } + + public void updateSellIn() { + this.item.sellIn -= 1; + } +} diff --git a/Java/src/main/java/com/gildedrose/GildedRose.java b/Java/src/main/java/com/gildedrose/GildedRose.java index e6feb751..962e3ddf 100644 --- a/Java/src/main/java/com/gildedrose/GildedRose.java +++ b/Java/src/main/java/com/gildedrose/GildedRose.java @@ -2,61 +2,19 @@ package com.gildedrose; class GildedRose { Item[] items; + ItemHolder[] itemHolders; public GildedRose(Item[] items) { this.items = items; + this.itemHolders = new ItemHolder[items.length]; + for (int i=0; i 0) { - if (!items[i].name.equals("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.equals("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.equals("Sulfuras, Hand of Ragnaros")) { - items[i].sellIn = items[i].sellIn - 1; - } - - if (items[i].sellIn < 0) { - if (!items[i].name.equals("Aged Brie")) { - if (!items[i].name.equals("Backstage passes to a TAFKAL80ETC concert")) { - if (items[i].quality > 0) { - if (!items[i].name.equals("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; - } - } - } + itemHolders[i].update(); } } -} \ No newline at end of file +} diff --git a/Java/src/main/java/com/gildedrose/ItemHolder.java b/Java/src/main/java/com/gildedrose/ItemHolder.java new file mode 100644 index 00000000..be0c3a0e --- /dev/null +++ b/Java/src/main/java/com/gildedrose/ItemHolder.java @@ -0,0 +1,17 @@ +package com.gildedrose; + +public abstract class ItemHolder { + Item item; + + public ItemHolder(Item item){ + this.item = item; + } + + public void update() { + this.updateQuality(); + this.updateSellIn(); + } + abstract void updateQuality(); + + abstract void updateSellIn(); +} diff --git a/Java/src/main/java/com/gildedrose/ItemHolderFactory.java b/Java/src/main/java/com/gildedrose/ItemHolderFactory.java new file mode 100644 index 00000000..58c77358 --- /dev/null +++ b/Java/src/main/java/com/gildedrose/ItemHolderFactory.java @@ -0,0 +1,15 @@ +package com.gildedrose; + +public class ItemHolderFactory { + public static ItemHolder createItemHolder(Item item){ + if (item.name == "Aged Brie") { + return new BrieHolder(item); + } else if (item.name == "Sulfuras, Hand of Ragnaros") { + return new SulfurasHolder(item); + } else if (item.name == "Backstage passes to a TAFKAL80ETC concert") { + return new BackstagePassesHolder(item); + } else { + return new GenericItemHolder(item); + } + } +} diff --git a/Java/src/main/java/com/gildedrose/SulfurasHolder.java b/Java/src/main/java/com/gildedrose/SulfurasHolder.java new file mode 100644 index 00000000..3f8c1461 --- /dev/null +++ b/Java/src/main/java/com/gildedrose/SulfurasHolder.java @@ -0,0 +1,11 @@ +package com.gildedrose; + +public class SulfurasHolder extends ItemHolder { + + public SulfurasHolder(Item item) { + super(item); + } + public void updateQuality() {} + + public void updateSellIn() {} +} diff --git a/Java/src/test/java/com/gildedrose/GildedRoseTest.java b/Java/src/test/java/com/gildedrose/GildedRoseTest.java index 8ae29eec..6164b08c 100644 --- a/Java/src/test/java/com/gildedrose/GildedRoseTest.java +++ b/Java/src/test/java/com/gildedrose/GildedRoseTest.java @@ -7,11 +7,129 @@ import static org.junit.jupiter.api.Assertions.assertEquals; class GildedRoseTest { @Test - void foo() { + void add_item() { Item[] items = new Item[] { new Item("foo", 0, 0) }; GildedRose app = new GildedRose(items); + assertEquals("foo", app.items[0].name); + assertEquals(0, app.items[0].sellIn); + assertEquals(0, app.items[0].quality); + } + + @Test + void quality_decrease() { + Item[] items = new Item[] { new Item("foo", 1, 40) }; + GildedRose app = new GildedRose(items); app.updateQuality(); - assertEquals("fixme", app.items[0].name); + assertEquals(0, app.items[0].sellIn); + assertEquals(39, app.items[0].quality); + } + + @Test + void fast_quality_decrease() { + Item[] items = new Item[] { new Item("foo", 0, 40) }; + GildedRose app = new GildedRose(items); + app.updateQuality(); + assertEquals(-1, app.items[0].sellIn); + assertEquals(38, app.items[0].quality); + } + + @Test + void never_negative_quality() { + Item[] items = new Item[] { new Item("foo", -1, 1) }; + GildedRose app = new GildedRose(items); + app.updateQuality(); + assertEquals(-2, app.items[0].sellIn); + assertEquals(0, app.items[0].quality); + } + + @Test + void aged_brie_quality_increase() { + Item[] items = new Item[] { new Item("Aged Brie", 5, 20) }; + GildedRose app = new GildedRose(items); + app.updateQuality(); + assertEquals(4, app.items[0].sellIn); + assertEquals(21, app.items[0].quality); + } + + @Test + void aged_brie_max_quality_increase() { + Item[] items = new Item[] { new Item("Aged Brie", 5, 50) }; + GildedRose app = new GildedRose(items); + app.updateQuality(); + assertEquals(4, app.items[0].sellIn); + assertEquals(50, app.items[0].quality); + } + + @Test + void sulfuras() { + Item[] items = new Item[] { new Item("Sulfuras, Hand of Ragnaros", 5, 80) }; + GildedRose app = new GildedRose(items); + app.updateQuality(); + assertEquals(5, app.items[0].sellIn); + assertEquals(80, app.items[0].quality); + } + + @Test + void sulfuras_expired() { + Item[] items = new Item[] { new Item("Sulfuras, Hand of Ragnaros", -2, 80) }; + GildedRose app = new GildedRose(items); + app.updateQuality(); + assertEquals(-2, app.items[0].sellIn); + assertEquals(80, app.items[0].quality); + } + + @Test + void backstage_passes_quality_increase() { + Item[] items = new Item[] { new Item("Backstage passes to a TAFKAL80ETC concert", 15, 20) }; + GildedRose app = new GildedRose(items); + app.updateQuality(); + assertEquals(14, app.items[0].sellIn); + assertEquals(21, app.items[0].quality); + } + + @Test + void backstage_passes_max_quality() { + Item[] items = new Item[] { new Item("Backstage passes to a TAFKAL80ETC concert", 2, 50) }; + GildedRose app = new GildedRose(items); + app.updateQuality(); + assertEquals(1, app.items[0].sellIn); + assertEquals(50, app.items[0].quality); + } + + @Test + void backstage_passes_day_ten() { + Item[] items = new Item[] { new Item("Backstage passes to a TAFKAL80ETC concert", 10, 20) }; + GildedRose app = new GildedRose(items); + app.updateQuality(); + assertEquals(9, app.items[0].sellIn); + assertEquals(22, app.items[0].quality); + } + + @Test + void backstage_passes_day_five() { + Item[] items = new Item[] { new Item("Backstage passes to a TAFKAL80ETC concert", 5, 20) }; + GildedRose app = new GildedRose(items); + app.updateQuality(); + assertEquals(4, app.items[0].sellIn); + assertEquals(23, app.items[0].quality); + } + + @Test + void backstage_passes_day_zero() { + Item[] items = new Item[] { new Item("Backstage passes to a TAFKAL80ETC concert", 1, 20) }; + GildedRose app = new GildedRose(items); + app.updateQuality(); + assertEquals(0, app.items[0].sellIn); + assertEquals(23, app.items[0].quality); + } + + @Test + void backstage_passes_expired() { + Item[] items = new Item[] { new Item("Backstage passes to a TAFKAL80ETC concert", 0, 20) }; + GildedRose app = new GildedRose(items); + app.updateQuality(); + assertEquals(-1, app.items[0].sellIn); + assertEquals(0, app.items[0].quality); } } diff --git a/Java/src/test/java/com/gildedrose/ItemHolderFactoryTest.java b/Java/src/test/java/com/gildedrose/ItemHolderFactoryTest.java new file mode 100644 index 00000000..f8e65d32 --- /dev/null +++ b/Java/src/test/java/com/gildedrose/ItemHolderFactoryTest.java @@ -0,0 +1,120 @@ +package com.gildedrose; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +class ItemHolderFactoryTest { + + @Test + void create_generic_item() { + ItemHolder itemHolder = ItemHolderFactory.createItemHolder(new Item("foo", 0, 0)); + assertEquals("foo", itemHolder.item.name); + assertEquals(0, itemHolder.item.sellIn); + assertEquals(0, itemHolder.item.quality); + } + + @Test + void quality_decrease() { + ItemHolder itemHolder = ItemHolderFactory.createItemHolder(new Item("foo", 1, 40)); + itemHolder.update(); + assertEquals(0, itemHolder.item.sellIn); + assertEquals(39, itemHolder.item.quality); + } + + @Test + void fast_quality_decrease() { + ItemHolder itemHolder = ItemHolderFactory.createItemHolder(new Item("foo", 0, 40)); + itemHolder.update(); + assertEquals(-1, itemHolder.item.sellIn); + assertEquals(38, itemHolder.item.quality); + } + + @Test + void never_negative_quality() { + ItemHolder itemHolder = ItemHolderFactory.createItemHolder(new Item("foo", -1, 1)); + itemHolder.update(); + assertEquals(-2, itemHolder.item.sellIn); + assertEquals(0, itemHolder.item.quality); + } + + @Test + void aged_brie_quality_increase() { + ItemHolder itemHolder = ItemHolderFactory.createItemHolder(new Item("Aged Brie", 5, 20)); + itemHolder.update(); + assertEquals(4, itemHolder.item.sellIn); + assertEquals(21, itemHolder.item.quality); + } + + @Test + void aged_brie_max_quality_increase() { + ItemHolder itemHolder = ItemHolderFactory.createItemHolder(new Item("Aged Brie", 5, 50)); + itemHolder.update(); + assertEquals(4, itemHolder.item.sellIn); + assertEquals(50, itemHolder.item.quality); + } + + @Test + void sulfuras() { + ItemHolder itemHolder = ItemHolderFactory.createItemHolder(new Item("Sulfuras, Hand of Ragnaros", 5, 80)); + itemHolder.update(); + assertEquals(5, itemHolder.item.sellIn); + assertEquals(80, itemHolder.item.quality); + } + + @Test + void sulfuras_expired() { + ItemHolder itemHolder = ItemHolderFactory.createItemHolder(new Item("Sulfuras, Hand of Ragnaros", -2, 80)); + itemHolder.update(); + assertEquals(-2, itemHolder.item.sellIn); + assertEquals(80, itemHolder.item.quality); + } + + @Test + void backstage_passes_quality_increase() { + ItemHolder itemHolder = ItemHolderFactory.createItemHolder(new Item("Backstage passes to a TAFKAL80ETC concert", 15, 20)); + itemHolder.update(); + assertEquals(14, itemHolder.item.sellIn); + assertEquals(21, itemHolder.item.quality); + } + + @Test + void backstage_passes_max_quality() { + ItemHolder itemHolder = ItemHolderFactory.createItemHolder(new Item("Backstage passes to a TAFKAL80ETC concert", 2, 50)); + itemHolder.update(); + assertEquals(1, itemHolder.item.sellIn); + assertEquals(50, itemHolder.item.quality); + } + + @Test + void backstage_passes_day_ten() { + ItemHolder itemHolder = ItemHolderFactory.createItemHolder(new Item("Backstage passes to a TAFKAL80ETC concert", 10, 20)); + itemHolder.update(); + assertEquals(9, itemHolder.item.sellIn); + assertEquals(22, itemHolder.item.quality); + } + + @Test + void backstage_passes_day_five() { + ItemHolder itemHolder = ItemHolderFactory.createItemHolder(new Item("Backstage passes to a TAFKAL80ETC concert", 5, 20)); + itemHolder.update(); + assertEquals(4, itemHolder.item.sellIn); + assertEquals(23, itemHolder.item.quality); + } + + @Test + void backstage_passes_day_zero() { + ItemHolder itemHolder = ItemHolderFactory.createItemHolder(new Item("Backstage passes to a TAFKAL80ETC concert", 1, 20)); + itemHolder.update(); + assertEquals(0, itemHolder.item.sellIn); + assertEquals(23, itemHolder.item.quality); + } + + @Test + void backstage_passes_expired() { + ItemHolder itemHolder = ItemHolderFactory.createItemHolder(new Item("Backstage passes to a TAFKAL80ETC concert", 0, 20)); + itemHolder.update(); + assertEquals(-1, itemHolder.item.sellIn); + assertEquals(0, itemHolder.item.quality); + } +}