Hi,
I was playing with Dcover, and I though I found a cool use case for the tool from a talk I remembered watching.
The talk is around the Gilded Rose Kata. In essence one has a blob of poorly designed legacy code with complex logic which that is undocumented. There are a series of items which are updated in different ways by a series of nested if/else statements. The ‘goal’ is to add support for a new item - of course, the real purpose of this goal is to make you realise that the nasty if stuff should be refactored into nice object oriented code (as is done in the video). This is a pretty good metaphor for what we see in enterprise legacy systems.
Of course, before refactoring, I would want to write a solid test suite to have confidence in my changes. Aside from the fact that I might not have great confidence in a test suite that I write given that I don’t understand the code, I thought it might be quicker to just let Dcover do it for me.
When I run Dcover on the project (after I translated it to Java, of course, I’ve included the key code) then the outcome is pretty good. There are three items which require special treatment - Aged Brie, Backstage Passes to a TAFKAL80ETC Concert, and Sulfuras, Hand of Ragnaros - and all others have some default behaviour. The tests from Dcover capture the behaviour of Aged Brie and Backstage Passes to a TAFKAL80ETC Concert correctly, along with the ‘default’ cases.
It was interesting to note that it missed the behaviour of the item Sulfuras, Hand of Ragnaros. The catch with this one (as also surprises the speaker in the talk) is that this item is never changed in any way by the update. I wonder, is there anything I can do here to help Dcover ‘see’ this behaviour and catch this case? Anyone have any tips?
Thanks!
p.s. I recommend everyone who is interested in good object oriented design take a look at more videos by the speaker (Sandi Metz) in the one I linked, she’s a great speaker and educator!
p.p.s it’s a shame I can’t upload Java code files!
Item Class
public class Item {
public String name;
public int sellIn;
public int quality;
public Item(final String name, final int sellIn, final int quality) {
this.name = name;
this.sellIn = sellIn;
this.quality = quality;
}
@Override
public String toString() {
return String.format("{name=%s, sellIn=%d, quality=%d}", name, sellIn, quality);
}
}
Items
public Collection<Item> items() {
return Collections.unmodifiableCollection(
Stream.of(
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("Backstage Passes to a TAFKAL80ETC Concert", 15, 20),
new Item("Conjured Mana Cake", 3, 6)
).collect(Collectors.toList())
);
}
Horrible, spaghetti code for updating items
public void updateQuality(final Item item) {
if (!"Aged Brie".equals(item.name)
&& !"Backstage passes to a TAFKAL80ETC concert".equals(item.name)) {
if (item.quality > 0) {
if (!"Sulfuras, Hand of Ragnaros".equals(item.name)) {
item.quality = item.quality - 1;
}
}
} else {
if (item.quality < 50) {
item.quality = item.quality + 1;
if (!"Backstage passes to a TAFKAL80ETC concert".equals(item.name)) {
if (item.sellIn < 11) {
if (item.quality < 50) {
item.quality = item.quality + 1;
}
}
if (item.sellIn < 6) {
if (item.quality < 50) {
item.quality = item.quality + 1;
}
}
}
}
}
if (!"Sulfuras, Hand of Ragnaros".equals(item.name)) {
item.sellIn = item.sellIn - 1;
}
if (item.sellIn < 0) {
if (!"Aged Brie".equals(item.name)) {
if (!"Backstage passes to a TAFKAL80ETC concert".equals(item.name)) {
if (item.quality > 0) {
if (!"Sulfuras, Hand of Ragnaros".equals(item.name)) {
item.quality = item.quality - 1;
}
}
} else {
item.quality = item.quality - item.quality;
}
} else {
if (item.quality < 50) {
item.quality = item.quality + 1;
}
}
}
}