From a8c6107f430d68cdab580e6c37b801f38e264ed1 Mon Sep 17 00:00:00 2001 From: Kadir Sirimsi Date: Tue, 11 Feb 2025 14:40:13 +0100 Subject: [PATCH] refactor: Move towards a better OOP design --- .../java/com/gildedrose/AgedBrieItem.java | 19 +++ .../com/gildedrose/BackstagePassItem.java | 27 ++++ Java/src/main/java/com/gildedrose/Item.java | 75 +++-------- .../main/java/com/gildedrose/ItemFactory.java | 16 +++ .../main/java/com/gildedrose/NormalItem.java | 19 +++ .../java/com/gildedrose/SulfurasItem.java | 13 ++ .../java/com/gildedrose/ItemFactoryTest.java | 53 ++++++++ .../test/java/com/gildedrose/ItemTest.java | 120 +++++++++++------- .../java/com/gildedrose/ItemTypeTest.java | 39 ++++++ .../java/com/gildedrose/TexttestFixture.java | 31 +++-- 10 files changed, 293 insertions(+), 119 deletions(-) create mode 100644 Java/src/main/java/com/gildedrose/AgedBrieItem.java create mode 100644 Java/src/main/java/com/gildedrose/BackstagePassItem.java create mode 100644 Java/src/main/java/com/gildedrose/ItemFactory.java create mode 100644 Java/src/main/java/com/gildedrose/NormalItem.java create mode 100644 Java/src/main/java/com/gildedrose/SulfurasItem.java create mode 100644 Java/src/test/java/com/gildedrose/ItemFactoryTest.java create mode 100644 Java/src/test/java/com/gildedrose/ItemTypeTest.java diff --git a/Java/src/main/java/com/gildedrose/AgedBrieItem.java b/Java/src/main/java/com/gildedrose/AgedBrieItem.java new file mode 100644 index 00000000..a11aa32d --- /dev/null +++ b/Java/src/main/java/com/gildedrose/AgedBrieItem.java @@ -0,0 +1,19 @@ +package com.gildedrose; + +public final class AgedBrieItem extends Item { + + AgedBrieItem(String name, int sellIn, int quality) { + super(name, sellIn, quality); + } + + @Override + public void degrade() { + addQuality(1); + + sellIn--; + + if (sellIn < 0) { + addQuality(1); + } + } +} diff --git a/Java/src/main/java/com/gildedrose/BackstagePassItem.java b/Java/src/main/java/com/gildedrose/BackstagePassItem.java new file mode 100644 index 00000000..fec37b77 --- /dev/null +++ b/Java/src/main/java/com/gildedrose/BackstagePassItem.java @@ -0,0 +1,27 @@ +package com.gildedrose; + +public final class BackstagePassItem extends Item { + + BackstagePassItem(String name, int sellIn, int quality) { + super(name, sellIn, quality); + } + + @Override + public void degrade() { + addQuality(1); + + if (sellIn < 11) { + addQuality(1); + } + + if (sellIn < 6) { + addQuality(1); + } + + sellIn--; + + if (sellIn < 0) { + quality = 0; + } + } +} diff --git a/Java/src/main/java/com/gildedrose/Item.java b/Java/src/main/java/com/gildedrose/Item.java index 92b2423e..d80cfb1b 100644 --- a/Java/src/main/java/com/gildedrose/Item.java +++ b/Java/src/main/java/com/gildedrose/Item.java @@ -2,83 +2,42 @@ package com.gildedrose; import java.util.Objects; -import static com.gildedrose.ItemType.fromName; +import static java.util.Objects.hash; -public class Item { +// I think ideally I should do this with a sealed interface instead, but for the current available types of items +// a sealed class seemed more convenient to me (especially because of shared member variables and setter methods). +public sealed abstract class Item permits + AgedBrieItem, + BackstagePassItem, + NormalItem, + SulfurasItem { private final String name; - private final ItemType type; - public int sellIn; public int quality; - public Item(String name, int sellIn, int quality) { + Item(String name, int sellIn, int quality) { this.name = name; this.sellIn = sellIn; this.quality = quality; - this.type = fromName(name); } + public abstract void degrade(); + + public String getName() { return name; } - public void updateItem() { - switch (type) { - case AgedBrie -> updateAgedBrieItem(); - case BackstagePass -> updateBackstagePassItem(); - case Sulfuras -> {} - case Normal -> updateNormalItem(); - } - } - - private void updateNormalItem() { - sellIn--; - - subtractQuality(1); - - if (sellIn < 0) { - subtractQuality(1); - } - } - - private void updateBackstagePassItem() { - addQuality(1); - - if (sellIn < 11) { - addQuality(1); - } - - if (sellIn < 6) { - addQuality(1); - } - - sellIn--; - - if (sellIn < 0) { - quality = 0; - } - } - - private void updateAgedBrieItem() { - addQuality(1); - - sellIn--; - - if (sellIn < 0) { - addQuality(1); - } - } - - private void addQuality(int addition) { + protected void addQuality(int addition) { if ((quality + addition) <= 50) { quality += addition; } } - private void subtractQuality(int subtraction) { + protected void subtractQuality(int subtraction) { if ((quality - subtraction) >= 0) { quality -= subtraction; } @@ -92,12 +51,12 @@ public class Item { @Override public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) return false; - Item item = (Item) o; - return sellIn == item.sellIn && quality == item.quality && Objects.equals(name, item.name) && type == item.type; + var item = (Item) o; + return sellIn == item.sellIn && quality == item.quality && Objects.equals(name, item.name); } @Override public int hashCode() { - return Objects.hash(name, type, sellIn, quality); + return hash(name, sellIn, quality); } } diff --git a/Java/src/main/java/com/gildedrose/ItemFactory.java b/Java/src/main/java/com/gildedrose/ItemFactory.java new file mode 100644 index 00000000..6fb8fb66 --- /dev/null +++ b/Java/src/main/java/com/gildedrose/ItemFactory.java @@ -0,0 +1,16 @@ +package com.gildedrose; + +import static com.gildedrose.ItemType.fromName; + +public class ItemFactory { + public static Item createItem(String name, int sellIn, int quality) { + var itemType = fromName(name); + + return switch (itemType) { + case AgedBrie -> new AgedBrieItem(name, sellIn, quality); + case BackstagePass -> new BackstagePassItem(name, sellIn, quality); + case Sulfuras -> new SulfurasItem(name, sellIn, quality); + case Normal -> new NormalItem(name, sellIn, quality); + }; + } +} diff --git a/Java/src/main/java/com/gildedrose/NormalItem.java b/Java/src/main/java/com/gildedrose/NormalItem.java new file mode 100644 index 00000000..7ea0dcbb --- /dev/null +++ b/Java/src/main/java/com/gildedrose/NormalItem.java @@ -0,0 +1,19 @@ +package com.gildedrose; + +public final class NormalItem extends Item { + + NormalItem(String name, int sellIn, int quality) { + super(name, sellIn, quality); + } + + @Override + public void degrade() { + sellIn--; + + subtractQuality(1); + + if (sellIn < 0) { + subtractQuality(1); + } + } +} diff --git a/Java/src/main/java/com/gildedrose/SulfurasItem.java b/Java/src/main/java/com/gildedrose/SulfurasItem.java new file mode 100644 index 00000000..22dd3fa7 --- /dev/null +++ b/Java/src/main/java/com/gildedrose/SulfurasItem.java @@ -0,0 +1,13 @@ +package com.gildedrose; + +public final class SulfurasItem extends Item { + + SulfurasItem(String name, int sellIn, int quality) { + super(name, sellIn, quality); + } + + @Override + public void degrade() { + // do nothing + } +} diff --git a/Java/src/test/java/com/gildedrose/ItemFactoryTest.java b/Java/src/test/java/com/gildedrose/ItemFactoryTest.java new file mode 100644 index 00000000..b3aa6b6a --- /dev/null +++ b/Java/src/test/java/com/gildedrose/ItemFactoryTest.java @@ -0,0 +1,53 @@ +package com.gildedrose; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +class ItemFactoryTest { + + @Test + void givenAgedBrieName_whenCreateItem_thenReturnsAgedBrieItem() { + Item item = ItemFactory.createItem("Aged Brie", 10, 20); + assertTrue(item instanceof AgedBrieItem); + assertEquals("Aged Brie", item.getName()); + assertEquals(10, item.sellIn); + assertEquals(20, item.quality); + } + + @Test + void givenBackstagePassName_whenCreateItem_thenReturnsBackstagePassItem() { + Item item = ItemFactory.createItem("Backstage passes to a TAFKAL80ETC concert", 15, 30); + assertTrue(item instanceof BackstagePassItem); + assertEquals("Backstage passes to a TAFKAL80ETC concert", item.getName()); + assertEquals(15, item.sellIn); + assertEquals(30, item.quality); + } + + @Test + void givenSulfurasName_whenCreateItem_thenReturnsSulfurasItem() { + Item item = ItemFactory.createItem("Sulfuras, Hand of Ragnaros", 0, 80); + assertTrue(item instanceof SulfurasItem); + assertEquals("Sulfuras, Hand of Ragnaros", item.getName()); + assertEquals(0, item.sellIn); + assertEquals(80, item.quality); + } + + @Test + void givenNormalItemName_whenCreateItem_thenReturnsNormalItem() { + Item item = ItemFactory.createItem("Some Normal Item", 5, 10); + assertTrue(item instanceof NormalItem); + assertEquals("Some Normal Item", item.getName()); + assertEquals(5, item.sellIn); + assertEquals(10, item.quality); + } + + @Test + void givenUnknownItemName_whenCreateItem_thenReturnsNormalItem() { + Item item = ItemFactory.createItem("Unknown Item", 5, 10); + assertTrue(item instanceof NormalItem); + assertEquals("Unknown Item", item.getName()); + assertEquals(5, item.sellIn); + assertEquals(10, item.quality); + } +} diff --git a/Java/src/test/java/com/gildedrose/ItemTest.java b/Java/src/test/java/com/gildedrose/ItemTest.java index c873496f..687a569a 100644 --- a/Java/src/test/java/com/gildedrose/ItemTest.java +++ b/Java/src/test/java/com/gildedrose/ItemTest.java @@ -1,62 +1,88 @@ package com.gildedrose; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.api.Test; -import java.util.stream.Stream; - -import static com.gildedrose.ItemType.AgedBrie; -import static com.gildedrose.ItemType.BackstagePass; -import static com.gildedrose.ItemType.Sulfuras; -import static com.gildedrose.ItemType.Normal; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.params.provider.Arguments.of; class ItemTest { - @ParameterizedTest - @MethodSource("provideAgedBrieOptions") - void givenAgedBrie_whenUpdateItem_thenCorrect(Item input, Item expected, int days) { - - for (int i = 0; i < days; i++) { - input.updateItem(); - } - - assertEquals(expected, input); + @Test + void givenNormalItem_whenDegrade_thenQualityAndSellInDecrease() { + var normalItem = new NormalItem("Normal Item", 10, 20); + normalItem.degrade(); + assertEquals(9, normalItem.sellIn); + assertEquals(19, normalItem.quality); } - private static Stream provideAgedBrieOptions() { - return Stream.of( + @Test + void givenAgedBrie_whenDegrade_thenQualityIncreases() { + var brie = new AgedBrieItem("Aged Brie", 10, 30); + brie.degrade(); + assertEquals(9, brie.sellIn); + assertEquals(31, brie.quality); + } - // Aged Brie - of(new Item(AgedBrie.getName(), -1, 0), new Item(AgedBrie.getName(), -2, 2), 1), - of(new Item(AgedBrie.getName(), 100, 0), new Item(AgedBrie.getName(), 0, 50), 100), - of(new Item(AgedBrie.getName(), 50, 0), new Item(AgedBrie.getName(), 40, 10), 10), - of(new Item(AgedBrie.getName(), 50, 0), new Item(AgedBrie.getName(), 20, 30), 30), + @Test + void givenBackstagePass_whenDegrade_thenQualityIncreasesBeforeSellDate() { + var pass = new BackstagePassItem("Backstage Pass", 11, 20); + pass.degrade(); + assertEquals(10, pass.sellIn); + assertEquals(21, pass.quality); + } - // Sulfuras -- no changes with sulfuras - of(new Item(Sulfuras.getName(), -1, 0), new Item(Sulfuras.getName(), -1, 0), 1), - of(new Item(Sulfuras.getName(), 10, 0), new Item(Sulfuras.getName(), 10, 0), 1), - of(new Item(Sulfuras.getName(), 100, 0), new Item(Sulfuras.getName(), 100, 0), 10), - of(new Item(Sulfuras.getName(), 10, 20), new Item(Sulfuras.getName(), 10, 20), 10), - of(new Item(Sulfuras.getName(), 4, 20), new Item(Sulfuras.getName(), 4, 20), 10), + @Test + void givenBackstagePass_whenSellInIs10OrLess_thenQualityIncreasesMore() { + var pass = new BackstagePassItem("Backstage Pass", 10, 20); + pass.degrade(); + assertEquals(9, pass.sellIn); + assertEquals(22, pass.quality); + } - // Backstagepass - of(new Item(BackstagePass.getName(), -1, 0), new Item(BackstagePass.getName(), -2, 0), 1), - of(new Item(BackstagePass.getName(), 8, 40), new Item(BackstagePass.getName(), 7, 42), 1), - of(new Item(BackstagePass.getName(), 8, 40), new Item(BackstagePass.getName(), -2, 0), 10), - of(new Item(BackstagePass.getName(), 4, 40), new Item(BackstagePass.getName(), -6, 0), 10), - of(new Item(BackstagePass.getName(), 0, 40), new Item(BackstagePass.getName(), -10, 0), 10), - of(new Item(BackstagePass.getName(), -1, 40), new Item(BackstagePass.getName(), -11, 0), 10), + @Test + void givenBackstagePass_whenSellInIsZero_thenQualityDropsToZero() { + var pass = new BackstagePassItem("Backstage Pass", 0, 20); + pass.degrade(); + assertEquals(-1, pass.sellIn); + assertEquals(0, pass.quality); + } - // Normal - of(new Item(Normal.getName(), -1, 0), new Item(Normal.getName(), -11, 0), 10), - of(new Item(Normal.getName(), 0, 40), new Item(Normal.getName(), -10, 20), 10), - of(new Item(Normal.getName(), -1, 40), new Item(Normal.getName(), -11, 20), 10), - of(new Item(Normal.getName(), 10, 49), new Item(Normal.getName(), -10, 19), 20), - of(new Item(Normal.getName(), 10, 49), new Item(Normal.getName(), -10, 19), 20), - of(new Item(Normal.getName(), 11, 50), new Item(Normal.getName(), -9, 21), 20) - ); + @Test + void givenSulfuras_whenDegrade_thenQualityAndSellInRemainUnchanged() { + var sulfuras = new SulfurasItem("Sulfuras, Hand of Ragnaros", 10, 80); + sulfuras.degrade(); + assertEquals(10, sulfuras.sellIn); + assertEquals(80, sulfuras.quality); + } + + @Test + void givenAgedBrie_whenQualityIs50_thenQualityDoesNotIncrease() { + var brie = new AgedBrieItem("Aged Brie", 10, 50); + brie.degrade(); + assertEquals(9, brie.sellIn); + assertEquals(50, brie.quality); + } + + @Test + void givenNormalItem_whenQualityIsZero_thenQualityDoesNotDecrease() { + var normalItem = new NormalItem("Normal Item", 10, 0); + normalItem.degrade(); + assertEquals(9, normalItem.sellIn); + assertEquals(0, normalItem.quality); + } + + @Test + void givenNormalItem_whenSellInIsNegative_thenQualityDecreasesTwice() { + var normalItem = new NormalItem("Normal Item", 0, 10); + normalItem.degrade(); + assertEquals(-1, normalItem.sellIn); + assertEquals(8, normalItem.quality); + } + + @Test + void givenAgedBrie_whenSellInIsNegative_thenQualityIncreasesTwice() { + var brie = new AgedBrieItem("Aged Brie", 0, 10); + brie.degrade(); + assertEquals(-1, brie.sellIn); + assertEquals(12, brie.quality); } } diff --git a/Java/src/test/java/com/gildedrose/ItemTypeTest.java b/Java/src/test/java/com/gildedrose/ItemTypeTest.java new file mode 100644 index 00000000..038c5d8d --- /dev/null +++ b/Java/src/test/java/com/gildedrose/ItemTypeTest.java @@ -0,0 +1,39 @@ +package com.gildedrose; + +import org.junit.jupiter.api.Test; + +import static com.gildedrose.ItemType.AgedBrie; +import static com.gildedrose.ItemType.BackstagePass; +import static com.gildedrose.ItemType.Normal; +import static com.gildedrose.ItemType.Sulfuras; +import static com.gildedrose.ItemType.fromName; +import static org.junit.jupiter.api.Assertions.*; + +class ItemTypeTest { + + @Test + void testGetName() { + assertEquals("Aged Brie", AgedBrie.getName()); + assertEquals("Backstage passes to a TAFKAL80ETC concert", BackstagePass.getName()); + assertEquals("Sulfuras, Hand of Ragnaros", Sulfuras.getName()); + assertEquals("Normal", Normal.getName()); + } + + @Test + void testFromName_ValidNames() { + assertEquals(AgedBrie, fromName("Aged Brie")); + assertEquals(BackstagePass, fromName("Backstage passes to a TAFKAL80ETC concert")); + assertEquals(Sulfuras, fromName("Sulfuras, Hand of Ragnaros")); + assertEquals(Normal, fromName("Normal")); + } + + @Test + void testFromName_InvalidName() { + assertEquals(Normal, fromName("Nonexistent Item")); + } + + @Test + void testFromName_NullInput() { + assertEquals(Normal, fromName(null)); + } +} diff --git a/Java/src/test/java/com/gildedrose/TexttestFixture.java b/Java/src/test/java/com/gildedrose/TexttestFixture.java index d059c88f..1a853632 100644 --- a/Java/src/test/java/com/gildedrose/TexttestFixture.java +++ b/Java/src/test/java/com/gildedrose/TexttestFixture.java @@ -1,36 +1,39 @@ package com.gildedrose; +import static com.gildedrose.ItemFactory.createItem; +import static java.lang.Integer.parseInt; + public class TexttestFixture { public static void main(String[] args) { System.out.println("OMGHAI!"); - Item[] items = new Item[] { - new Item("+5 Dexterity Vest", 10, 20), // - new Item("Aged Brie", 2, 0), // - new Item("Elixir of the Mongoose", 5, 7), // - new Item("Sulfuras, Hand of Ragnaros", 0, 80), // - new Item("Sulfuras, Hand of Ragnaros", -1, 80), - new Item("Backstage passes to a TAFKAL80ETC concert", 15, 20), - new Item("Backstage passes to a TAFKAL80ETC concert", 10, 49), - new Item("Backstage passes to a TAFKAL80ETC concert", 5, 49), + var items = new Item[] { + createItem("+5 Dexterity Vest", 10, 20), // + createItem("Aged Brie", 2, 0), // + createItem("Elixir of the Mongoose", 5, 7), // + createItem("Sulfuras, Hand of Ragnaros", 0, 80), // + createItem("Sulfuras, Hand of Ragnaros", -1, 80), + createItem("Backstage passes to a TAFKAL80ETC concert", 15, 20), + createItem("Backstage passes to a TAFKAL80ETC concert", 10, 49), + createItem("Backstage passes to a TAFKAL80ETC concert", 5, 49), // this conjured item does not work properly yet - new Item("Conjured Mana Cake", 3, 6) }; + createItem("Conjured Mana Cake", 3, 6) }; - GildedRose app = new GildedRose(items); + var app = new GildedRose(items); int days = 2; if (args.length > 0) { - days = Integer.parseInt(args[0]) + 1; + days = parseInt(args[0]) + 1; } for (int i = 0; i < days; i++) { System.out.println("-------- day " + i + " --------"); System.out.println("name, sellIn, quality"); - for (Item item : items) { + for (var item : items) { System.out.println(item); } System.out.println(); - app.updateQuality(); + app.degradeItems(); } }