https://github.com/wouterla/GildedRose/
and all of us where jumping between solutions as with deep in the code.
When i go back to home i decided to did that kata with an stricted procedure , and this is the results, i had expend 5 pomodoros of 30 minutes
The flow was :
You could follow all my keystrokes with the file of MoonEdit soft , KataGildedRose
- prepare test over Item Class
- prepare test for GildeRose Class.
- replace magic number with symbolic constant
- write code to complete the conjured item test
- consolidate conditional expresions
- extract method for increments , decrements of values, and expired()
- Split conditionals, substitute else, split nested if clauses, change sucesive if with && operator.
- noTopQuality() and hasQuality() conditions, send to increaseQuality and decreaseQuality method couse they always are checked before those methods
- solve double negations
- erase inclusive at sentences with exclusive clauses ( if (isA) && (!isB) ==> so isA
- try to extract common clauses, (ad !sufuras if posible) then extract it
- extract method of each item behaviour
- join the code of conjured item to the new behaviour sistem
- replace conditional with polimorphism
- prepare test over Item Class
download the file
import junit.framework.TestCase;
public class ItemTest extends TestCase {
//All items have a SellIn value
public void testHasSellInValue()
{
int expectedResult =23;
int myItemSellIn=23;
int myItemQuality = 55;
Item myItem= new Item("myName",myItemSellIn,myItemQuality);
assertEquals(expectedResult, myItem.getSellIn());
}
//All items have a Quality value
public void testHasQualityValue()
{
int expectedResult =55;
int myItemSellIn=23;
int myItemQuality = 55;
Item myItem= new Item("myName",myItemSellIn,myItemQuality);
assertEquals(expectedResult, myItem.getQuality());
}
//At the end of each day our system lowers both values for every item
public void testCouldBeDecreasedSellInValue()
{
int expectedResult =22;
int myItemSellIn=23;
int myItemQuality = 55;
Item myItem= new Item("myName",myItemSellIn,myItemQuality);
myItem.setSellIn(myItem.getSellIn()-1);
assertEquals(expectedResult, myItem.getSellIn());
}
public void testCouldBeDecreasedQuality()
{
int expectedResult =54;
int myItemSellIn=23;
int myItemQuality = 55;
Item myItem= new Item("myName",myItemSellIn,myItemQuality);
myItem.setQuality(myItem.getQuality()-1);
assertEquals(expectedResult, myItem.getQuality());
}
} - prepare test for GildeRose Class. here i make a little change to avoid use of reflection for test private fields. i 'd change items from private to public scope
download the testClass file of GildedRose
import junit.framework.TestCase;
public class GildedRoseTest extends TestCase {
private static final int DEXTERY_VEST = 0;
private static final int AGED_BRIE = 1;
private static final int ELIXIR_MONGOOSE = 2;
private static final int SULFURAS = 3;
private static final int BACKSTAGE_PASSES = 4;
private static final int CONJURED = 5;
// Once the sell by date has passed, Quality degrades twice as fast
public void testDegradeTwiceWhenSellInDatePassed (){
initializeGildedRose();
int valueBeforeLastNormalDegradation =0;
int valueAfterLastNormalDegradation =0;
while (GildedRose.items.get(DEXTERY_VEST).getSellIn()>0) {
valueBeforeLastNormalDegradation = GildedRose.items.get(DEXTERY_VEST).getQuality();
GildedRose.updateQuality();
valueAfterLastNormalDegradation = GildedRose.items.get(DEXTERY_VEST).getQuality();
}
int qualityNormalDegradationRate = valueBeforeLastNormalDegradation-valueAfterLastNormalDegradation;
int qualityBeforeDegradeAfterSellInPassed = GildedRose.items.get(DEXTERY_VEST).getQuality();
GildedRose.updateQuality();
assertEquals(qualityBeforeDegradeAfterSellInPassed - (qualityNormalDegradationRate*2),GildedRose.items.get(DEXTERY_VEST).getQuality());
}
// The Quality of an item is never negative
public void testNeverNegativeQuality(){
initializeGildedRose();
updateTillReachQuality(ELIXIR_MONGOOSE, 0);
GildedRose.updateQuality();
assertTrue(GildedRose.items.get(ELIXIR_MONGOOSE).getQuality()>=0);
}
// "Aged Brie" actually increases in Quality the older it gets
public void testAgedBrieIncrementsQualiyAtUpdate(){
initializeGildedRose();
int beforeQuality = GildedRose.items.get(AGED_BRIE).getQuality();
GildedRose.updateQuality();
int afterQuality = GildedRose.items.get(AGED_BRIE).getQuality();
assertTrue(afterQuality>beforeQuality);
}
// The Quality of an item is never more than 50
public void testqualityNeverMoreThanFifty(){
initializeGildedRose();
updateTillReachQuality(AGED_BRIE, 45);
for (int timesToGuaranteeAsMuchAsCanFifty = 0; timesToGuaranteeAsMuchAsCanFifty < 5; timesToGuaranteeAsMuchAsCanFifty++) { GildedRose.updateQuality(); } GildedRose.updateQuality(); assertTrue(GildedRose.items.get(AGED_BRIE).getQuality()<=50); } // "Sulfuras", being a legendary item, never has to be sold or decreases in Quality public void testSulfurasNoSellInAndNoLooseQuality(){ initializeGildedRose(); int beforeQuality = GildedRose.items.get(SULFURAS).getQuality(); int beforeSellIn = GildedRose.items.get(SULFURAS).getSellIn(); GildedRose.updateQuality(); int afterQuality = GildedRose.items.get(SULFURAS).getQuality(); int afterSellIn = GildedRose.items.get(SULFURAS).getSellIn(); assertTrue(beforeSellIn == afterSellIn); assertTrue(beforeSellIn==0); assertTrue(afterQuality==beforeQuality); } // "Backstage passes", like aged brie, increases in Quality as it's SellIn value approaches; Quality increases by 2 when there are 10 days or less and by 3 when there are 5 days or less but Quality drops to 0 after the concert public void testBackStageBehaviour(){ initializeGildedRose(); updateTillSellIn(BACKSTAGE_PASSES,11); int qualityBeforeAnUpdateToTenDays = GildedRose.items.get(BACKSTAGE_PASSES).getQuality(); GildedRose.updateQuality(); int qualityAfterAnUpdateToTenDays = GildedRose.items.get(BACKSTAGE_PASSES).getQuality(); updateTillSellIn(BACKSTAGE_PASSES,6); int qualityBeforeAnUpdateToFiveDays = GildedRose.items.get(BACKSTAGE_PASSES).getQuality(); GildedRose.updateQuality(); int qualityAfterAnUpdateToFiveDays = GildedRose.items.get(BACKSTAGE_PASSES).getQuality(); updateTillSellIn(BACKSTAGE_PASSES,1); GildedRose.updateQuality(); int qualityCeroDaysLeft = GildedRose.items.get(BACKSTAGE_PASSES).getQuality(); int qualityGapAtTen = qualityAfterAnUpdateToTenDays-qualityBeforeAnUpdateToTenDays; int qualityGapAtFive = qualityAfterAnUpdateToFiveDays - qualityBeforeAnUpdateToFiveDays; assertEquals(qualityGapAtTen, 2); assertEquals(qualityGapAtFive, 3); assertTrue(qualityCeroDaysLeft==0); } // "Conjured" items degrade in Quality twice as fast as normal items public void testConjuredItemDegradesTwiceAsNormal (){ initializeGildedRose(); int valueBeforeLastNormalDegradation = GildedRose.items.get(DEXTERY_VEST).getQuality(); int valueBeforeDegradation = GildedRose.items.get(CONJURED).getQuality(); GildedRose.updateQuality(); int valueAfterLastNormalDegradation = GildedRose.items.get(DEXTERY_VEST).getQuality(); int valueAfterDegradation = GildedRose.items.get(CONJURED).getQuality(); int qualityNormalDegradationRate = valueBeforeLastNormalDegradation - valueAfterLastNormalDegradation; int qualityConjuredDegradationRate = valueBeforeDegradation - valueAfterDegradation; assertEquals(qualityNormalDegradationRate*2 , qualityConjuredDegradationRate); } public void testConjuredDegradeTwiceWhenSellInDatePassed (){ initializeGildedRose(); int valueBeforeLastNormalDegradation =0; int valueAfterLastNormalDegradation =0; while (GildedRose.items.get(CONJURED).getSellIn()>=0) {
valueBeforeLastNormalDegradation = GildedRose.items.get(CONJURED).getQuality();
GildedRose.updateQuality();
valueAfterLastNormalDegradation = GildedRose.items.get(CONJURED).getQuality();
}
int qualityNormalDegradationRate = valueBeforeLastNormalDegradation-valueAfterLastNormalDegradation;
int qualityBeforeDegradeAfterSellInPassed = GildedRose.items.get(CONJURED).getQuality();
GildedRose.updateQuality();
assertEquals(qualityBeforeDegradeAfterSellInPassed - (qualityNormalDegradationRate*2),GildedRose.items.get(CONJURED).getQuality());
}
private void updateTillSellIn(int itemNumber, int sellInBottom) {
while (GildedRose.items.get(itemNumber).getSellIn() >= sellInBottom) {
GildedRose.updateQuality();
}
}
private void initializeGildedRose() {
GildedRose.main(null);
}
private void updateTillReachQuality(int itemNumber, int qualityToReach) {
while (GildedRose.items.get(itemNumber).getQuality() <= qualityToReach) { GildedRose.updateQuality(); } } } - replace magic number with symbolic constant
- write code to complete the conjured item test
- consolidate conditional expresions
- extract method for increments , decrements of values, and expired()
after this we gat the code
import java.util.ArrayList;
import java.util.List;
public class GildedRose {
private static List- items = null;
/**
* @param args
*/
private final static String DEXTERY_VEST = "+5 Dexterity Vest";
private final static String AGED_BRIE = "Aged Brie";
private final static String ELIXIR_MONGOOSE = "Elixir of the Mongoose";
private final static String SULFURAS = "Sulfuras, Hand of Ragnaros";
private final static String BACKSTAGE_PASSES = "Backstage passes to a TAFKAL80ETC concert";
private final static String CONJURED = "Conjured Mana Cake";
private final static int MAX_QUALITY = 50;
private final static int MIN_QUALITY = 0;
public static void main(String[] args) {
System.out.println("OMGHAI!");
items = new ArrayList- ();
items.add(new Item(DEXTERY_VEST, 10, 20));
items.add(new Item(AGED_BRIE, 2, 0));
items.add(new Item(ELIXIR_MONGOOSE, 5, 7));
items.add(new Item(SULFURAS, 0, 80));
items.add(new Item(BACKSTAGE_PASSES, 15, 20));
items.add(new Item("Conjured Mana Cake", 3, 6));
updateQuality();
}
public static void updateQuality()
{
int conjuredQualityBeforeUpdate = getConjuredQuality();
for (int i = 0; i < items.size(); i++) { if ((!isAged(i) && !isBackStage(i)) { if (hasQuality(i)) { if (!isSulfuras(i)) { decrementQuality(i); } } } else { if (noTopQuality(i)) { incrementQuality(i); if (isBackStage(i)) { if (items.get(i).getSellIn() < 11) { if (notTopQuality(i)) { incrementQuality(i); } } if (items.get(i).getSellIn() < 6) { if (noTopQuality(i)) { incrementQuality(i); } } } } } if (!isSulfuras(i)) { decrementSellIn(i); } if (expired(i)) { if (!isAged(i)) { if (!isBackStage(i)) { if (hasQuality(i)) { if (!isSulfuras(i)) { decrementQuality(i); } } } else { looseAllQuality(i); } } else { if (noTopQuality(i)) { incrementQuality(i); } } } } int conjuredQualityAfterUpdate = getConjuredQuality(); int normalItemQualityChange = conjuredQualityAfterUpdate - conjuredQualityBeforeUpdate); setConjuredQuality(getConjuredQuality() + normalItemQualityChange); } private static int getConjuredQuality(){ return getQualityByName(CONJURED); } private static void setConjuredQuality(int amount){ setConjuredQualityByName(CONJURED,amount); } private static void setConjuredQualityByName(String name,int amount){ for (int i = 0; i < items.size(); i++) { if (name.equals(items.get(i).getName())) items.get(i).setQuality(amount); } } private static int getQualityByName(String name){ for (int i = 0; i < items.size(); i++) { if (name.equals(items.get(i).getName())) return items.get(i).getQuality(); } return 0; } private static boolean isAged(int i) { return AGED_BRIE.equals(getNameByIndex(i)); } private static boolean isBackStage(int i) { return BACKSTAGE_PASSES.equals(getNameByIndex(i)); } private static boolean isSulfuras(int i) { return SULFURAS.equals(getNameByIndex(i)); } private static String getNameByIndex(int i) { return items.get(i).getName(); } private static void decrementQuality(int i) { changeQuality (i,- 1); } private static void incrementQuality(int i) { changeQuality(i,1); } private static void changeQuality(int i,int amount) { items.get(i).setQuality(items.get(i).getQuality() + amount); } private static boolean hasQuality(int i) { return (items.get(i).getQuality() > MIN_QUALITY);
}
private static boolean noTopQuality(int i)
{
return (items.get(i).getQuality() < MAX_QUALITY); } private static void looseAllQuality(int i) { items.get(i).setQuality(MIN_QUALITY); } private static void decrementSellIn(int i) { items.get(i).setSellIn(items.get(i).getSellIn() - 1); } private static boolean expired(int i) { return (items.get(i).getSellIn() < 0); } } - items = null;
- Split conditionals, substitute else, split nested if clauses, change sucesive if with && operator.
- noTopQuality() and hasQuality() conditions, send to increaseQuality and decreaseQuality method couse they always are checked before those methods
- solve double negations
- erase inclusive at sentences with exclusive clauses ( if (isA) && (!isB) ==> so isA
- try to extract common clauses, (ad !sufuras if posible) then extract it
- extract method of each item behaviour
- join the code of conjured item to the new behaviour sistem
- replace conditional with polimorphism
- items = null;
/**
* @param args
*/
private final static String DEXTERY_VEST = "+5 Dexterity Vest";
private final static String AGED_BRIE = "Aged Brie";
private final static String ELIXIR_MONGOOSE = "Elixir of the Mongoose";
private final static String SULFURAS = "Sulfuras, Hand of Ragnaros";
private final static String BACKSTAGE_PASSES = "Backstage passes to a TAFKAL80ETC concert";
private final static String CONJURED = "Conjured Mana Cake";
//
public static void main(String[] args) {
System.out.println("OMGHAI!");
items = new ArrayList- ();
items.add(new Item(DEXTERY_VEST, 10, 20));
items.add(new Item(AGED_BRIE, 2, 0));
items.add(new Item(ELIXIR_MONGOOSE, 5, 7));
items.add(new Item(SULFURAS, 0, 80));
items.add(new Item(BACKSTAGE_PASSES, 15, 20));
items.add(new Item("Conjured Mana Cake", 3, 6));
updateQuality();
}
public static void updateQuality()
{
for (int i = 0; i < items.size(); i++) { if (!isSulfuras) { Article itemToUpdate = factoriaDeItems(i); itemToUpdate.decrementSellIn(); itemToUpdate.updateQuality(); copyValues(items.get(i),itemToUpdate); } } } private static void copyValues(Item item, Article itemToUpdate) { item.setQuality(itemToUpdate.getQuality()); item.setSellIn(itemToUpdate.getSellIn()); } private static Article factoriaDeItems(int i) { if (isAged(i)) return new AgedBrie(items.get(i)); if (isBackStage(i)) return new BackStage(items.get(i)); if (isConjured(i)) return new Conjured(items.get(i)); return new GenericArticle(items.get(i)); } private static boolean isAged(int i) { return AGED_BRIE.equals(getNameByIndex(i)); } private static boolean isBackStage(int i) { return BACKSTAGE_PASSES.equals(getNameByIndex(i)); } private static boolean isSulfuras(int i) { return SULFURAS.equals(getNameByIndex(i)); } private static boolean isConjured(int i) { return CONJURED.equals(getNameByIndex(i)); } private static String getNameByIndex(int i) { return items.get(i).getName(); } } public abstract class Article extends Item { private final static int MAX_QUALITY = 50; private final static int MIN_QUALITY = 0; public Article(String name, int sellIn, int quality) { super(name, sellIn, quality); } public Article (Item item) { super (item.getName(),item.getSellIn(), item.getQuality()); } public abstract void updateQuality(); protected void decrementSellIn() { setSellIn(getSellIn() - 1); } protected void decrementQuality() { if (hasQuality()) changeQuality (- 1); } protected void incrementQuality() { if (noTopQuality()) changeQuality(1); } protected void changeQuality(int amount) { setQuality(getQuality() + amount); } protected boolean hasQuality() { return (getQuality() > MIN_QUALITY);
}
protected boolean noTopQuality()
{
return (getQuality() < MAX_QUALITY); } protected void looseAllQuality() { setQuality(MIN_QUALITY); } protected boolean expired() { return (getSellIn() < 0); } } public class GenericArticle extends Article { public GenericArticle (Item item) { super (item); } public void updateQuality() { //extracted of method updateNormalItem() decrementQuality(); if (expired()) decrementQuality(); } } public class AgedBrie extends Article { public AgedBrie (Item item) { super (item); } public void updateQuality() { incrementQuality(); incrementQuality(); } } public class BackStage extends Article { public BackStage (Item item) { super (item); } public void updateQuality() { incrementQuality(); if ((getSellIn() < 11)) { incrementQuality(); } if ((getSellIn() < 6)) { incrementQuality(); } if (expired()) { looseAllQuality(); } } } public class Conjured extends Article { public Conjured (Item item) { super(item); } public void updateQuality() { decrementQuality(); decrementQuality(); if (expired()) { decrementQuality(); decrementQuality(); } } } - ();
this only the code changed
public static void updateQuality()
{
int conjuredQualityBeforeUpdate = getConjuredQuality();
for (int i = 0; i < items.size(); i++) { //hasquality() and no topQuality are quality pro if ((!isAged(i) && !isBackStage(i))&&(hasQuality(i))&&(!isSulfuras(i))) { decrementQuality(i); } if ((!((!isAged(i) && !isBackStage(i)))&&(noTopQuality(i)))) { incrementQuality(i); } if ((!((!isAged(i) && !isBackStage(i)))&&(noTopQuality(i))&&(isBackStage(i))&&(items.get(i).getSellIn() < 11)&&(noTopQuality(i)))) { incrementQuality(i); } if ((isBackStage(i))&&(items.get(i).getSellIn() < 6)&& (noTopQuality(i))) { incrementQuality(i); } if (!isSulfuras(i)) { decrementSellIn(i); } if ((expired(i))&&(!isAged(i))&&(!isBackStage(i))&&if (hasQuality(i))&&if (!isSulfuras(i))) { decrementQuality(i); } if (((expired(i)&&(!isAged(i)) &&(isBackStage(i)))) { looseAllQuality(i); } if ((isAged(i))&&(noTopQuality(i))) { incrementQuality(i); } } int conjuredQualityAfterUpdate = getConjuredQuality(); int normalItemQualityChange = conjuredQualityAfterUpdate - conjuredQualityBeforeUpdate; setConjuredQuality(getConjuredQuality() + normalItemQualityChange); }
public static void updateQuality()
{
int conjuredQualityBeforeUpdate = getConjuredQuality();
for (int i = 0; i < items.size(); i++) { //extrac condition common if (!isSulfuras(i))) { decrementSellIn(i); if (isAged(i) || isBackStage(i)) { incrementQuality(i); } else { decrementQuality(i); } if (isBackStage(i)) { if (expired(i) { looseAllQuality(i); } if ((items.get(i).getSellIn() < 11)) { incrementQuality(i); } if ((items.get(i).getSellIn() < 6)) { incrementQuality(i); } } if ((expired(i))&&(!isAged(i))&&(!isBackStage(i)) { decrementQuality(i); } if ((isAged(i))) { incrementQuality(i); } } } int conjuredQualityAfterUpdate = getConjuredQuality(); int normalItemQualityChange = conjuredQualityAfterUpdate - conjuredQualityBeforeUpdate; setConjuredQuality(getConjuredQuality() + normalItemQualityChange); }
public static void updateQuality()
{
for (int i = 0; i < items.size(); i++) { if (!isSulfuras(i)) { decrementSellIn(i); updateNormalItem(i); updateConjuredItem(i); if (isAged(i)) updateQualityAged(i); if (isBackStage(i)) updateQualityBackStage(i); } } } private static void updateConjuredItem(int i) { updateNormalItem(i); updateNormalItem(i); } private static void updateNormalItem (int i) { updateQualityNormalItem(i); if (expired(i)) updateQualityExpiredNormalItem(i); } private static void updateQualityExpiredNormalItem(int i) { if ((!isAged(i))&&(!isBackStage(i))) { decrementQuality(i); } } private static void updateQualityNormalItem(int i) { if (!isBackStage(i) && !isAged(i)) { decrementQuality(i); } } private static void updateQualityAged(int i) { incrementQuality(i); incrementQuality(i); } private static void updateQualityBackStage(int i) { incrementQuality(i); if (expired(i)) { looseAllQuality(i); } if ((items.get(i).getSellIn() < 11)) { incrementQuality(i); } if ((items.get(i).getSellIn() < 6)) { incrementQuality(i); } }
import java.util.ArrayList;
import java.util.List;
public class GildedRose {
private static List
read next post to see
from Is-a to Has-a and from extends to implements
