From fcb6c1d61d94b646628415f5561ccd2abb90ad36 Mon Sep 17 00:00:00 2001 From: Mohamed BEN HARIZ New laptop Date: Tue, 1 Apr 2025 14:10:32 +0200 Subject: [PATCH] refectoring the code and add readme file to explique the context --- csharp.NUnit/GildedRose/BaseUpdateStrategy.cs | 27 +++ csharp.NUnit/GildedRose/GildedRose.cs | 79 +------- csharp.NUnit/GildedRose/IUpdateStrategy.cs | 6 + csharp.NUnit/GildedRose/ItemCategory.cs | 13 ++ .../GildedRose/Strategies/AgedBrieStrategy.cs | 15 ++ .../Strategies/BackstagePassStrategy.cs | 26 +++ .../Strategies/ConjuredItemStrategy.cs | 16 ++ .../Strategies/StandardItemStrategy.cs | 15 ++ .../GildedRose/Strategies/SulfurasStrategy.cs | 10 + .../GildedRose/UpdateStrategyFactory.cs | 18 ++ .../GildedRoseTests/GildedRoseTest.cs | 140 +++++++++++++- csharp.NUnit/README.md | 178 ++++++++++++++++-- 12 files changed, 451 insertions(+), 92 deletions(-) create mode 100644 csharp.NUnit/GildedRose/BaseUpdateStrategy.cs create mode 100644 csharp.NUnit/GildedRose/IUpdateStrategy.cs create mode 100644 csharp.NUnit/GildedRose/ItemCategory.cs create mode 100644 csharp.NUnit/GildedRose/Strategies/AgedBrieStrategy.cs create mode 100644 csharp.NUnit/GildedRose/Strategies/BackstagePassStrategy.cs create mode 100644 csharp.NUnit/GildedRose/Strategies/ConjuredItemStrategy.cs create mode 100644 csharp.NUnit/GildedRose/Strategies/StandardItemStrategy.cs create mode 100644 csharp.NUnit/GildedRose/Strategies/SulfurasStrategy.cs create mode 100644 csharp.NUnit/GildedRose/UpdateStrategyFactory.cs diff --git a/csharp.NUnit/GildedRose/BaseUpdateStrategy.cs b/csharp.NUnit/GildedRose/BaseUpdateStrategy.cs new file mode 100644 index 00000000..e70173c9 --- /dev/null +++ b/csharp.NUnit/GildedRose/BaseUpdateStrategy.cs @@ -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); +} diff --git a/csharp.NUnit/GildedRose/GildedRose.cs b/csharp.NUnit/GildedRose/GildedRose.cs index c08bf5f8..be0e8d55 100644 --- a/csharp.NUnit/GildedRose/GildedRose.cs +++ b/csharp.NUnit/GildedRose/GildedRose.cs @@ -1,10 +1,10 @@ -using System.Collections.Generic; +using System.Collections.Generic; namespace GildedRoseKata; public class GildedRose { - IList Items; + private readonly IList Items; public GildedRose(IList 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); } } -} \ No newline at end of file +} diff --git a/csharp.NUnit/GildedRose/IUpdateStrategy.cs b/csharp.NUnit/GildedRose/IUpdateStrategy.cs new file mode 100644 index 00000000..91806355 --- /dev/null +++ b/csharp.NUnit/GildedRose/IUpdateStrategy.cs @@ -0,0 +1,6 @@ +namespace GildedRoseKata; + +public interface IUpdateStrategy +{ + void UpdateQuality(Item item); +} diff --git a/csharp.NUnit/GildedRose/ItemCategory.cs b/csharp.NUnit/GildedRose/ItemCategory.cs new file mode 100644 index 00000000..c829505d --- /dev/null +++ b/csharp.NUnit/GildedRose/ItemCategory.cs @@ -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; +} diff --git a/csharp.NUnit/GildedRose/Strategies/AgedBrieStrategy.cs b/csharp.NUnit/GildedRose/Strategies/AgedBrieStrategy.cs new file mode 100644 index 00000000..7053835b --- /dev/null +++ b/csharp.NUnit/GildedRose/Strategies/AgedBrieStrategy.cs @@ -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); + } + } +} diff --git a/csharp.NUnit/GildedRose/Strategies/BackstagePassStrategy.cs b/csharp.NUnit/GildedRose/Strategies/BackstagePassStrategy.cs new file mode 100644 index 00000000..7d12e762 --- /dev/null +++ b/csharp.NUnit/GildedRose/Strategies/BackstagePassStrategy.cs @@ -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; + } + } +} diff --git a/csharp.NUnit/GildedRose/Strategies/ConjuredItemStrategy.cs b/csharp.NUnit/GildedRose/Strategies/ConjuredItemStrategy.cs new file mode 100644 index 00000000..a01f0442 --- /dev/null +++ b/csharp.NUnit/GildedRose/Strategies/ConjuredItemStrategy.cs @@ -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); + } + } +} diff --git a/csharp.NUnit/GildedRose/Strategies/StandardItemStrategy.cs b/csharp.NUnit/GildedRose/Strategies/StandardItemStrategy.cs new file mode 100644 index 00000000..8cf7a8ea --- /dev/null +++ b/csharp.NUnit/GildedRose/Strategies/StandardItemStrategy.cs @@ -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); + } + } +} diff --git a/csharp.NUnit/GildedRose/Strategies/SulfurasStrategy.cs b/csharp.NUnit/GildedRose/Strategies/SulfurasStrategy.cs new file mode 100644 index 00000000..587670ff --- /dev/null +++ b/csharp.NUnit/GildedRose/Strategies/SulfurasStrategy.cs @@ -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; + } +} diff --git a/csharp.NUnit/GildedRose/UpdateStrategyFactory.cs b/csharp.NUnit/GildedRose/UpdateStrategyFactory.cs new file mode 100644 index 00000000..5d4600bb --- /dev/null +++ b/csharp.NUnit/GildedRose/UpdateStrategyFactory.cs @@ -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() + }; + } +} diff --git a/csharp.NUnit/GildedRoseTests/GildedRoseTest.cs b/csharp.NUnit/GildedRoseTests/GildedRoseTest.cs index 6f7c585c..1f2b44f5 100644 --- a/csharp.NUnit/GildedRoseTests/GildedRoseTest.cs +++ b/csharp.NUnit/GildedRoseTests/GildedRoseTest.cs @@ -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 { new Item { Name = "foo", SellIn = 0, Quality = 0 } }; + var items = new List { 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)); } -} \ No newline at end of file + + [Test] + public void StandardItem_QualityDegradesTwiceAsFastAfterSellIn() + { + var items = new List { 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 { 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 { 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 { 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 { 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 { 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 { 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 { 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 { 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 { 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 + { + 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)); + } +} diff --git a/csharp.NUnit/README.md b/csharp.NUnit/README.md index 8e1eaea9..b4019d79 100644 --- a/csharp.NUnit/README.md +++ b/csharp.NUnit/README.md @@ -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 { + <> + +UpdateQuality(Item) + } + + class BaseUpdateStrategy { + <> + #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 -``` \ No newline at end of file +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