From a585b181da712ac3ac6d49cb92717ae3b05edcf3 Mon Sep 17 00:00:00 2001 From: Sarah Ashri Date: Thu, 14 Mar 2024 14:27:42 +1000 Subject: [PATCH] Move DailyUpdate of each type to DailyUpdaterHierarchy and add a simple factory method to get the appropriate updater to use. --- csharpcore/GildedRose/DailyUpdater.cs | 94 +++++++++++++++++++++++ csharpcore/GildedRose/GildedRose.cs | 60 +++------------ csharpcore/GildedRose/RefactoringDiary.md | 11 ++- 3 files changed, 113 insertions(+), 52 deletions(-) create mode 100644 csharpcore/GildedRose/DailyUpdater.cs diff --git a/csharpcore/GildedRose/DailyUpdater.cs b/csharpcore/GildedRose/DailyUpdater.cs new file mode 100644 index 00000000..a9a4aec3 --- /dev/null +++ b/csharpcore/GildedRose/DailyUpdater.cs @@ -0,0 +1,94 @@ +using System.Data; + +namespace GildedRoseKata; + +public abstract class DailyUpdater +{ + protected const int MinQuality = 0; + protected const int MaxQuality = 50; + + public void DailyUpdate(Item item) + { + UpdateSellIn(item); + UpdateQuality(item); + } + + public abstract void UpdateQuality(Item item); + public virtual void UpdateSellIn(Item item) => item.SellIn -= 1; + + protected static bool IsExpired(Item item) => item.SellIn < 0; + + protected static void IncreaseQuality(Item item, int byValue) + { + item.Quality = int.Min(item.Quality + byValue, MaxQuality); + } + + protected static void DecreaseQuality(Item item, int byValue) + { + item.Quality = int.Max(item.Quality - byValue, MinQuality); + } + + +} + +public class DailyUpdaterForRegularItems : DailyUpdater +{ + public override void UpdateQuality(Item item) + { + DecreaseQuality(item, 1); + if (IsExpired(item)) + { + DecreaseQuality(item, 1); + } + } +} + +public class DailyUpdaterForBetterWithAgeItems : DailyUpdater +{ + public override void UpdateQuality(Item item) + { + IncreaseQuality(item, 1); + if (IsExpired(item)) + { + IncreaseQuality(item, 1); + } + } +} + + +public class DailyUpdaterForBackstagePassesItems : DailyUpdater +{ + public override void UpdateQuality(Item item) + { + if (item.SellIn > 9) + { + IncreaseQuality(item, 1); + } + else if (item.SellIn > 4) + { + IncreaseQuality(item, 2); + } + else if (!IsExpired(item)) + { + IncreaseQuality(item, 3); + } + else //Expired + { + DecreaseQuality(item, item.Quality); + } + } +} + +public class DailyUpdaterForLegendaryItems : DailyUpdater +{ + public override void UpdateSellIn(Item item) + { + // Legendary Items don't change over time + } + public override void UpdateQuality(Item item) + { + // Legendary Items don't change over time + } +} + + diff --git a/csharpcore/GildedRose/GildedRose.cs b/csharpcore/GildedRose/GildedRose.cs index c10dac65..7abeccb5 100644 --- a/csharpcore/GildedRose/GildedRose.cs +++ b/csharpcore/GildedRose/GildedRose.cs @@ -17,7 +17,8 @@ public class GildedRose { foreach (var item in _items) { - DailyItemUpdate(item); + DailyUpdater dailyUpdater = getDailyUpdater(item); + dailyUpdater.DailyUpdate(item); } } @@ -26,67 +27,24 @@ public class GildedRose private static bool IsBackstagePassesItem(Item item) => item.Name.ToLower().Contains("backstage passes"); private static bool IsBetterWithAgeItem(Item item) => item.Name.ToLower().Equals("aged brie"); - - private static bool IsRegularItem(Item item) => (!IsLegendaryItem(item) && - !IsBackstagePassesItem(item) && - !IsBetterWithAgeItem(item)); - - private static bool IsExpired(Item item) => item.SellIn < 0; - private static void IncreaseQuality(Item item, int byValue) + private DailyUpdater getDailyUpdater(Item item) { - item.Quality = int.Min(item.Quality + byValue, MaxQuality); - } - - private static void DecreaseQuality(Item item, int byValue) - { - item.Quality = int.Max(item.Quality - byValue, MinQuality); - } - - - private void DailyItemUpdate(Item item) - { - if (IsLegendaryItem(item)) return; - - item.SellIn -= 1; - - if (IsRegularItem(item)) + if (IsLegendaryItem(item)) { - DecreaseQuality(item, 1); - if (IsExpired(item)) - { - DecreaseQuality(item, 1); - } + return new DailyUpdaterForLegendaryItems(); } if (IsBetterWithAgeItem(item)) { - IncreaseQuality(item, 1); - if (IsExpired(item)) - { - IncreaseQuality(item, 1); - } + return new DailyUpdaterForBetterWithAgeItems(); } - if(IsBackstagePassesItem(item)) + if (IsBackstagePassesItem(item)) { - if (item.SellIn > 9) - { - IncreaseQuality(item, 1); - } - else if (item.SellIn > 4) - { - IncreaseQuality(item, 2); - } - else if (!IsExpired(item)) - { - IncreaseQuality(item, 3); - } - else //Expired - { - DecreaseQuality(item, item.Quality); - } + return new DailyUpdaterForBackstagePassesItems(); } + return new DailyUpdaterForRegularItems(); } } \ No newline at end of file diff --git a/csharpcore/GildedRose/RefactoringDiary.md b/csharpcore/GildedRose/RefactoringDiary.md index 5d602418..a70da823 100644 --- a/csharpcore/GildedRose/RefactoringDiary.md +++ b/csharpcore/GildedRose/RefactoringDiary.md @@ -12,11 +12,20 @@ Some of the tests failed due to bugs I discovered in the code/existing tests (e. I decorated these tests with `[Ignore]` until I get to a stage in the refactoring where I can easily fix the issues and re-introduce the tests. 2. small refactorings of the UpdateQuality() method to make it more readable and separate the different (unrelated) handling of the different types.
3. Extract Increasing/Decreasing Quality into a method.
-It feels like Quality should have been a TinyType that manages its own limits. However, since we're not allowed to change Item, I'm just extracting a method. 4. Separated the processing of the different types from each other.
+5. Used the `Template` design pattern to create DailyUpdater abstract class and derived classes to handle the DailyUpdate's steps for the different types of items.
+The different derived classes will be moved to their own files in the next PR. Left them together for now to show the steps. +6. Created a `Simple Factory` method to return the appropriate DailyUpdater object for the item type.
+For this PR, I left this method in the GildedRose.cp to show the steps. But in the next PR I'll move it into its own class. This is the state of the code now +### Considered but dropped +This is all the design ideas/patterns I've considered but decided against +- It feels like Quality should have been a `TinyType` that manages its own limits. However, since we're not allowed to change Item I'm not implementing this. +- + +