mirror of
https://github.com/emilybache/GildedRose-Refactoring-Kata.git
synced 2026-02-07 18:52:19 +00:00
refectoring the code and add readme file to explique the context
This commit is contained in:
parent
b3a0463bba
commit
fcb6c1d61d
27
csharp.NUnit/GildedRose/BaseUpdateStrategy.cs
Normal file
27
csharp.NUnit/GildedRose/BaseUpdateStrategy.cs
Normal file
@ -0,0 +1,27 @@
|
||||
namespace GildedRoseKata;
|
||||
|
||||
public abstract class BaseUpdateStrategy : IUpdateStrategy
|
||||
{
|
||||
protected static void DecreaseQuality(Item item)
|
||||
{
|
||||
if (item.Quality > ItemCategory.MinQuality)
|
||||
{
|
||||
item.Quality--;
|
||||
}
|
||||
}
|
||||
|
||||
protected static void IncreaseQuality(Item item)
|
||||
{
|
||||
if (item.Quality < ItemCategory.MaxQuality)
|
||||
{
|
||||
item.Quality++;
|
||||
}
|
||||
}
|
||||
|
||||
protected static void DecreaseSellIn(Item item)
|
||||
{
|
||||
item.SellIn--;
|
||||
}
|
||||
|
||||
public abstract void UpdateQuality(Item item);
|
||||
}
|
||||
@ -1,10 +1,10 @@
|
||||
using System.Collections.Generic;
|
||||
using System.Collections.Generic;
|
||||
|
||||
namespace GildedRoseKata;
|
||||
|
||||
public class GildedRose
|
||||
{
|
||||
IList<Item> Items;
|
||||
private readonly IList<Item> Items;
|
||||
|
||||
public GildedRose(IList<Item> Items)
|
||||
{
|
||||
@ -13,77 +13,10 @@ public class GildedRose
|
||||
|
||||
public void UpdateQuality()
|
||||
{
|
||||
for (var i = 0; i < Items.Count; i++)
|
||||
foreach (var item in Items)
|
||||
{
|
||||
if (Items[i].Name != "Aged Brie" && Items[i].Name != "Backstage passes to a TAFKAL80ETC concert")
|
||||
{
|
||||
if (Items[i].Quality > 0)
|
||||
{
|
||||
if (Items[i].Name != "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 == "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 != "Sulfuras, Hand of Ragnaros")
|
||||
{
|
||||
Items[i].SellIn = Items[i].SellIn - 1;
|
||||
}
|
||||
|
||||
if (Items[i].SellIn < 0)
|
||||
{
|
||||
if (Items[i].Name != "Aged Brie")
|
||||
{
|
||||
if (Items[i].Name != "Backstage passes to a TAFKAL80ETC concert")
|
||||
{
|
||||
if (Items[i].Quality > 0)
|
||||
{
|
||||
if (Items[i].Name != "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;
|
||||
}
|
||||
}
|
||||
}
|
||||
var strategy = UpdateStrategyFactory.CreateStrategy(item.Name);
|
||||
strategy.UpdateQuality(item);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
6
csharp.NUnit/GildedRose/IUpdateStrategy.cs
Normal file
6
csharp.NUnit/GildedRose/IUpdateStrategy.cs
Normal file
@ -0,0 +1,6 @@
|
||||
namespace GildedRoseKata;
|
||||
|
||||
public interface IUpdateStrategy
|
||||
{
|
||||
void UpdateQuality(Item item);
|
||||
}
|
||||
13
csharp.NUnit/GildedRose/ItemCategory.cs
Normal file
13
csharp.NUnit/GildedRose/ItemCategory.cs
Normal file
@ -0,0 +1,13 @@
|
||||
namespace GildedRoseKata;
|
||||
|
||||
public static class ItemCategory
|
||||
{
|
||||
public const string AgedBrie = "Aged Brie";
|
||||
public const string BackstagePasses = "Backstage passes to a TAFKAL80ETC concert";
|
||||
public const string Sulfuras = "Sulfuras, Hand of Ragnaros";
|
||||
public const string Conjured = "Conjured Mana Cake";
|
||||
|
||||
public const int MaxQuality = 50;
|
||||
public const int MinQuality = 0;
|
||||
public const int LegendaryQuality = 80;
|
||||
}
|
||||
15
csharp.NUnit/GildedRose/Strategies/AgedBrieStrategy.cs
Normal file
15
csharp.NUnit/GildedRose/Strategies/AgedBrieStrategy.cs
Normal file
@ -0,0 +1,15 @@
|
||||
namespace GildedRoseKata.Strategies;
|
||||
|
||||
public class AgedBrieStrategy : BaseUpdateStrategy
|
||||
{
|
||||
public override void UpdateQuality(Item item)
|
||||
{
|
||||
IncreaseQuality(item);
|
||||
DecreaseSellIn(item);
|
||||
|
||||
if (item.SellIn < 0)
|
||||
{
|
||||
IncreaseQuality(item);
|
||||
}
|
||||
}
|
||||
}
|
||||
26
csharp.NUnit/GildedRose/Strategies/BackstagePassStrategy.cs
Normal file
26
csharp.NUnit/GildedRose/Strategies/BackstagePassStrategy.cs
Normal file
@ -0,0 +1,26 @@
|
||||
namespace GildedRoseKata.Strategies;
|
||||
|
||||
public class BackstagePassStrategy : BaseUpdateStrategy
|
||||
{
|
||||
public override void UpdateQuality(Item item)
|
||||
{
|
||||
IncreaseQuality(item);
|
||||
|
||||
if (item.SellIn <= 10)
|
||||
{
|
||||
IncreaseQuality(item);
|
||||
}
|
||||
|
||||
if (item.SellIn <= 5)
|
||||
{
|
||||
IncreaseQuality(item);
|
||||
}
|
||||
|
||||
DecreaseSellIn(item);
|
||||
|
||||
if (item.SellIn < 0)
|
||||
{
|
||||
item.Quality = ItemCategory.MinQuality;
|
||||
}
|
||||
}
|
||||
}
|
||||
16
csharp.NUnit/GildedRose/Strategies/ConjuredItemStrategy.cs
Normal file
16
csharp.NUnit/GildedRose/Strategies/ConjuredItemStrategy.cs
Normal file
@ -0,0 +1,16 @@
|
||||
namespace GildedRoseKata.Strategies;
|
||||
|
||||
public class ConjuredItemStrategy : BaseUpdateStrategy
|
||||
{
|
||||
public override void UpdateQuality(Item item)
|
||||
{
|
||||
// For now, Conjured items degrade at normal rate to match original behavior
|
||||
DecreaseQuality(item);
|
||||
DecreaseSellIn(item);
|
||||
|
||||
if (item.SellIn < 0)
|
||||
{
|
||||
DecreaseQuality(item);
|
||||
}
|
||||
}
|
||||
}
|
||||
15
csharp.NUnit/GildedRose/Strategies/StandardItemStrategy.cs
Normal file
15
csharp.NUnit/GildedRose/Strategies/StandardItemStrategy.cs
Normal file
@ -0,0 +1,15 @@
|
||||
namespace GildedRoseKata.Strategies;
|
||||
|
||||
public class StandardItemStrategy : BaseUpdateStrategy
|
||||
{
|
||||
public override void UpdateQuality(Item item)
|
||||
{
|
||||
DecreaseQuality(item);
|
||||
DecreaseSellIn(item);
|
||||
|
||||
if (item.SellIn < 0)
|
||||
{
|
||||
DecreaseQuality(item);
|
||||
}
|
||||
}
|
||||
}
|
||||
10
csharp.NUnit/GildedRose/Strategies/SulfurasStrategy.cs
Normal file
10
csharp.NUnit/GildedRose/Strategies/SulfurasStrategy.cs
Normal file
@ -0,0 +1,10 @@
|
||||
namespace GildedRoseKata.Strategies;
|
||||
|
||||
public class SulfurasStrategy : BaseUpdateStrategy
|
||||
{
|
||||
public override void UpdateQuality(Item item)
|
||||
{
|
||||
// Sulfuras is legendary and never changes
|
||||
item.Quality = ItemCategory.LegendaryQuality;
|
||||
}
|
||||
}
|
||||
18
csharp.NUnit/GildedRose/UpdateStrategyFactory.cs
Normal file
18
csharp.NUnit/GildedRose/UpdateStrategyFactory.cs
Normal file
@ -0,0 +1,18 @@
|
||||
namespace GildedRoseKata;
|
||||
|
||||
using GildedRoseKata.Strategies;
|
||||
|
||||
public static class UpdateStrategyFactory
|
||||
{
|
||||
public static IUpdateStrategy CreateStrategy(string itemName)
|
||||
{
|
||||
return itemName switch
|
||||
{
|
||||
ItemCategory.AgedBrie => new AgedBrieStrategy(),
|
||||
ItemCategory.BackstagePasses => new BackstagePassStrategy(),
|
||||
ItemCategory.Sulfuras => new SulfurasStrategy(),
|
||||
ItemCategory.Conjured => new ConjuredItemStrategy(),
|
||||
_ => new StandardItemStrategy()
|
||||
};
|
||||
}
|
||||
}
|
||||
@ -1,4 +1,4 @@
|
||||
using System.Collections.Generic;
|
||||
using System.Collections.Generic;
|
||||
using GildedRoseKata;
|
||||
using NUnit.Framework;
|
||||
|
||||
@ -7,11 +7,141 @@ namespace GildedRoseTests;
|
||||
public class GildedRoseTest
|
||||
{
|
||||
[Test]
|
||||
public void Foo()
|
||||
public void StandardItem_QualityDegradesByOne()
|
||||
{
|
||||
var items = new List<Item> { new Item { Name = "foo", SellIn = 0, Quality = 0 } };
|
||||
var items = new List<Item> { new Item { Name = "Standard Item", SellIn = 5, Quality = 10 } };
|
||||
var app = new GildedRose(items);
|
||||
|
||||
app.UpdateQuality();
|
||||
Assert.That(items[0].Name, Is.EqualTo("fixme"));
|
||||
|
||||
Assert.That(items[0].Quality, Is.EqualTo(9));
|
||||
Assert.That(items[0].SellIn, Is.EqualTo(4));
|
||||
}
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void StandardItem_QualityDegradesTwiceAsFastAfterSellIn()
|
||||
{
|
||||
var items = new List<Item> { new Item { Name = "Standard Item", SellIn = 0, Quality = 10 } };
|
||||
var app = new GildedRose(items);
|
||||
|
||||
app.UpdateQuality();
|
||||
|
||||
Assert.That(items[0].Quality, Is.EqualTo(8));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void AgedBrie_IncreasesInQuality()
|
||||
{
|
||||
var items = new List<Item> { new Item { Name = ItemCategory.AgedBrie, SellIn = 5, Quality = 10 } };
|
||||
var app = new GildedRose(items);
|
||||
|
||||
app.UpdateQuality();
|
||||
|
||||
Assert.That(items[0].Quality, Is.EqualTo(11));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void AgedBrie_QualityNeverExceedsFifty()
|
||||
{
|
||||
var items = new List<Item> { new Item { Name = ItemCategory.AgedBrie, SellIn = 5, Quality = 50 } };
|
||||
var app = new GildedRose(items);
|
||||
|
||||
app.UpdateQuality();
|
||||
|
||||
Assert.That(items[0].Quality, Is.EqualTo(50));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Sulfuras_NeverChanges()
|
||||
{
|
||||
var items = new List<Item> { new Item { Name = ItemCategory.Sulfuras, SellIn = 5, Quality = 80 } };
|
||||
var app = new GildedRose(items);
|
||||
|
||||
app.UpdateQuality();
|
||||
|
||||
Assert.That(items[0].Quality, Is.EqualTo(80));
|
||||
Assert.That(items[0].SellIn, Is.EqualTo(5));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void BackstagePasses_IncreasesInQuality()
|
||||
{
|
||||
var items = new List<Item> { new Item { Name = ItemCategory.BackstagePasses, SellIn = 15, Quality = 20 } };
|
||||
var app = new GildedRose(items);
|
||||
|
||||
app.UpdateQuality();
|
||||
|
||||
Assert.That(items[0].Quality, Is.EqualTo(21));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void BackstagePasses_IncreasesByTwoWhenTenDaysOrLess()
|
||||
{
|
||||
var items = new List<Item> { new Item { Name = ItemCategory.BackstagePasses, SellIn = 10, Quality = 20 } };
|
||||
var app = new GildedRose(items);
|
||||
|
||||
app.UpdateQuality();
|
||||
|
||||
Assert.That(items[0].Quality, Is.EqualTo(22));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void BackstagePasses_IncreasesByThreeWhenFiveDaysOrLess()
|
||||
{
|
||||
var items = new List<Item> { new Item { Name = ItemCategory.BackstagePasses, SellIn = 5, Quality = 20 } };
|
||||
var app = new GildedRose(items);
|
||||
|
||||
app.UpdateQuality();
|
||||
|
||||
Assert.That(items[0].Quality, Is.EqualTo(23));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void BackstagePasses_QualityDropsToZeroAfterConcert()
|
||||
{
|
||||
var items = new List<Item> { new Item { Name = ItemCategory.BackstagePasses, SellIn = 0, Quality = 20 } };
|
||||
var app = new GildedRose(items);
|
||||
|
||||
app.UpdateQuality();
|
||||
|
||||
Assert.That(items[0].Quality, Is.EqualTo(0));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void ConjuredItem_DegradesAtNormalRate()
|
||||
{
|
||||
var items = new List<Item> { new Item { Name = ItemCategory.Conjured, SellIn = 5, Quality = 20 } };
|
||||
var app = new GildedRose(items);
|
||||
|
||||
app.UpdateQuality();
|
||||
|
||||
Assert.That(items[0].Quality, Is.EqualTo(19));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void ConjuredItem_DegradesAtNormalRateAfterSellIn()
|
||||
{
|
||||
var items = new List<Item> { new Item { Name = ItemCategory.Conjured, SellIn = 0, Quality = 20 } };
|
||||
var app = new GildedRose(items);
|
||||
|
||||
app.UpdateQuality();
|
||||
|
||||
Assert.That(items[0].Quality, Is.EqualTo(18));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void QualityIsNeverNegative()
|
||||
{
|
||||
var items = new List<Item>
|
||||
{
|
||||
new Item { Name = "Standard Item", SellIn = 5, Quality = 0 },
|
||||
new Item { Name = ItemCategory.Conjured, SellIn = 5, Quality = 0 }
|
||||
};
|
||||
var app = new GildedRose(items);
|
||||
|
||||
app.UpdateQuality();
|
||||
|
||||
Assert.That(items[0].Quality, Is.EqualTo(0));
|
||||
Assert.That(items[1].Quality, Is.EqualTo(0));
|
||||
}
|
||||
}
|
||||
|
||||
@ -1,24 +1,174 @@
|
||||
# Gilded Rose starting position in C# NUnit
|
||||
# GildedRose Refactoring Kata - C# Solution
|
||||
|
||||
## Build the project
|
||||
This solution demonstrates a refactoring of the GildedRose kata using clean code principles and design patterns.
|
||||
|
||||
Use your normal build tools to build the projects in Debug mode.
|
||||
For example, you can use the `dotnet` command line tool:
|
||||
## Design Patterns Used
|
||||
|
||||
``` cmd
|
||||
dotnet build GildedRose.sln -c Debug
|
||||
### Class Diagram
|
||||
```mermaid
|
||||
classDiagram
|
||||
class Item {
|
||||
+string Name
|
||||
+int SellIn
|
||||
+int Quality
|
||||
}
|
||||
|
||||
class GildedRose {
|
||||
-IList~Item~ Items
|
||||
+UpdateQuality()
|
||||
}
|
||||
|
||||
class IUpdateStrategy {
|
||||
<<interface>>
|
||||
+UpdateQuality(Item)
|
||||
}
|
||||
|
||||
class BaseUpdateStrategy {
|
||||
<<abstract>>
|
||||
#DecreaseQuality(Item)
|
||||
#IncreaseQuality(Item)
|
||||
#DecreaseSellIn(Item)
|
||||
+UpdateQuality(Item)*
|
||||
}
|
||||
|
||||
class UpdateStrategyFactory {
|
||||
+CreateStrategy(string) IUpdateStrategy$
|
||||
}
|
||||
|
||||
StandardItemStrategy --|> BaseUpdateStrategy
|
||||
AgedBrieStrategy --|> BaseUpdateStrategy
|
||||
BackstagePassStrategy --|> BaseUpdateStrategy
|
||||
SulfurasStrategy --|> BaseUpdateStrategy
|
||||
ConjuredItemStrategy --|> BaseUpdateStrategy
|
||||
|
||||
BaseUpdateStrategy ..|> IUpdateStrategy
|
||||
GildedRose --> IUpdateStrategy
|
||||
GildedRose --> UpdateStrategyFactory
|
||||
UpdateStrategyFactory ..> BaseUpdateStrategy
|
||||
```
|
||||
|
||||
## Run the Gilded Rose Command-Line program
|
||||
### 1. Strategy Pattern
|
||||
The main design pattern used in this refactoring is the Strategy Pattern, which enables:
|
||||
- Encapsulation of different item update algorithms
|
||||
- Easy addition of new item types
|
||||
- Elimination of complex conditional logic
|
||||
|
||||
For e.g. 10 days:
|
||||
The pattern is implemented through:
|
||||
- `IUpdateStrategy`: Interface defining the contract for all strategies
|
||||
- `BaseUpdateStrategy`: Abstract base class with common functionality
|
||||
- Concrete strategies for each item type:
|
||||
- `StandardItemStrategy`
|
||||
- `AgedBrieStrategy`
|
||||
- `BackstagePassStrategy`
|
||||
- `SulfurasStrategy`
|
||||
- `ConjuredItemStrategy`
|
||||
|
||||
``` cmd
|
||||
GildedRose/bin/Debug/net8.0/GildedRose 10
|
||||
### 2. Factory Pattern
|
||||
The Factory Pattern is used to create appropriate strategies:
|
||||
- `UpdateStrategyFactory`: Creates the correct strategy based on item name
|
||||
- Centralizes strategy instantiation
|
||||
- Makes it easy to add new item types
|
||||
|
||||
## Code Structure
|
||||
|
||||
```
|
||||
GildedRose/
|
||||
├── ItemCategory.cs # Constants and configuration
|
||||
├── IUpdateStrategy.cs # Strategy interface
|
||||
├── BaseUpdateStrategy.cs # Common strategy functionality
|
||||
├── Strategies/
|
||||
│ ├── StandardItemStrategy.cs
|
||||
│ ├── AgedBrieStrategy.cs
|
||||
│ ├── BackstagePassStrategy.cs
|
||||
│ ├── SulfurasStrategy.cs
|
||||
│ └── ConjuredItemStrategy.cs
|
||||
└── UpdateStrategyFactory.cs # Factory for creating strategies
|
||||
```
|
||||
|
||||
## Run all the unit tests
|
||||
## Key Improvements
|
||||
|
||||
``` cmd
|
||||
dotnet test
|
||||
```
|
||||
1. **Separation of Concerns**
|
||||
- Each item type has its own strategy class
|
||||
- Common functionality extracted to base class
|
||||
- Clear responsibility boundaries
|
||||
|
||||
2. **Maintainability**
|
||||
- Eliminated nested if statements
|
||||
- Removed code duplication
|
||||
- Centralized constants
|
||||
- Easy to modify individual item behaviors
|
||||
|
||||
3. **Extensibility**
|
||||
- Adding new item types only requires:
|
||||
1. Creating a new strategy class
|
||||
2. Adding item name to ItemCategory
|
||||
3. Adding strategy to factory
|
||||
- No modification of existing code needed (Open/Closed Principle)
|
||||
|
||||
4. **Testability**
|
||||
- Each strategy can be tested independently
|
||||
- Clear test cases for each item type
|
||||
- Easy to add tests for new items
|
||||
|
||||
## How to Use
|
||||
|
||||
1. **Adding a New Item Type**
|
||||
```csharp
|
||||
// 1. Add item name to ItemCategory
|
||||
public static class ItemCategory
|
||||
{
|
||||
public const string NewItem = "New Item Name";
|
||||
}
|
||||
|
||||
// 2. Create new strategy
|
||||
public class NewItemStrategy : BaseUpdateStrategy
|
||||
{
|
||||
public override void UpdateQuality(Item item)
|
||||
{
|
||||
// Implement update logic
|
||||
}
|
||||
}
|
||||
|
||||
// 3. Add to factory
|
||||
public static class UpdateStrategyFactory
|
||||
{
|
||||
public static IUpdateStrategy CreateStrategy(string itemName)
|
||||
{
|
||||
return itemName switch
|
||||
{
|
||||
ItemCategory.NewItem => new NewItemStrategy(),
|
||||
// ... existing cases
|
||||
};
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
2. **Modifying Item Behavior**
|
||||
- Locate the appropriate strategy class
|
||||
- Modify the UpdateQuality method
|
||||
- Add/update tests as needed
|
||||
|
||||
## Testing
|
||||
|
||||
The solution includes comprehensive tests:
|
||||
- Unit tests for each item type
|
||||
- Edge case testing
|
||||
- Approval tests for system behavior
|
||||
- Quality boundary tests
|
||||
- SellIn date tests
|
||||
|
||||
## Future Enhancements
|
||||
|
||||
The code is structured to easily accommodate future changes:
|
||||
- Double degradation for Conjured items
|
||||
- New item types
|
||||
- Modified quality rules
|
||||
- Additional item properties
|
||||
|
||||
## Design Principles Applied
|
||||
|
||||
- **Single Responsibility Principle**: Each class has one reason to change
|
||||
- **Open/Closed Principle**: Open for extension, closed for modification
|
||||
- **Dependency Inversion**: High-level modules depend on abstractions
|
||||
- **DRY (Don't Repeat Yourself)**: Common logic in base class
|
||||
- **KISS (Keep It Simple, Stupid)**: Clear, straightforward implementations
|
||||
|
||||
Loading…
Reference in New Issue
Block a user