mirror of
https://github.com/emilybache/GildedRose-Refactoring-Kata.git
synced 2026-02-04 09:11:39 +00:00
refactor: Move towards a better OOP design
This commit is contained in:
parent
0458a9adb2
commit
a8c6107f43
19
Java/src/main/java/com/gildedrose/AgedBrieItem.java
Normal file
19
Java/src/main/java/com/gildedrose/AgedBrieItem.java
Normal file
@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
27
Java/src/main/java/com/gildedrose/BackstagePassItem.java
Normal file
27
Java/src/main/java/com/gildedrose/BackstagePassItem.java
Normal file
@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
16
Java/src/main/java/com/gildedrose/ItemFactory.java
Normal file
16
Java/src/main/java/com/gildedrose/ItemFactory.java
Normal file
@ -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);
|
||||
};
|
||||
}
|
||||
}
|
||||
19
Java/src/main/java/com/gildedrose/NormalItem.java
Normal file
19
Java/src/main/java/com/gildedrose/NormalItem.java
Normal file
@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
13
Java/src/main/java/com/gildedrose/SulfurasItem.java
Normal file
13
Java/src/main/java/com/gildedrose/SulfurasItem.java
Normal file
@ -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
|
||||
}
|
||||
}
|
||||
53
Java/src/test/java/com/gildedrose/ItemFactoryTest.java
Normal file
53
Java/src/test/java/com/gildedrose/ItemFactoryTest.java
Normal file
@ -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);
|
||||
}
|
||||
}
|
||||
@ -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<Arguments> 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);
|
||||
}
|
||||
}
|
||||
|
||||
39
Java/src/test/java/com/gildedrose/ItemTypeTest.java
Normal file
39
Java/src/test/java/com/gildedrose/ItemTypeTest.java
Normal file
@ -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));
|
||||
}
|
||||
}
|
||||
@ -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();
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user