From 68ee4cc4cd0773cccb6ad37fa3fda2e3d43bdacd Mon Sep 17 00:00:00 2001 From: "@arik" <72645415+ArikAckerman@users.noreply.github.com> Date: Mon, 24 Apr 2023 02:00:56 +0300 Subject: [PATCH] Update GildedRose.cs In this refactored version, I've made the following changes: Changed the name of the private member variable to use camelCase naming convention and added a "readonly" keyword to make it immutable. Changed the loop to use a "foreach" loop instead of a "for" loop, which is more idiomatic in C#. Extracted the code for updating an individual item's quality into a separate private method to improve readability and reduce duplication. Removed unnecessary nested "if" statements by using "else if" and "else" statements. Extracted the code for increasing or decreasing an item's quality into separate private methods to improve readability and reduce duplication. Moved the logic for updating an item's sell-in value into a separate private method to improve readability and reduce duplication. --- csharp/GildedRose.cs | 130 +++++++++++++++++++++---------------------- 1 file changed, 64 insertions(+), 66 deletions(-) diff --git a/csharp/GildedRose.cs b/csharp/GildedRose.cs index c60d97a0..38c86919 100644 --- a/csharp/GildedRose.cs +++ b/csharp/GildedRose.cs @@ -1,89 +1,87 @@ -using System.Collections.Generic; +using System.Collections.Generic; namespace csharp { public class GildedRose { - IList Items; - public GildedRose(IList Items) + private readonly IList _items; + + public GildedRose(IList items) { - this.Items = Items; + _items = items; } 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") + UpdateItemQuality(item); + UpdateItemSellIn(item); + } + } + + private void UpdateItemQuality(Item item) + { + if (item.Name == "Aged Brie") + { + IncreaseQuality(item); + if (item.SellIn < 0) { - if (Items[i].Quality > 0) - { - if (Items[i].Name != "Sulfuras, Hand of Ragnaros") - { - Items[i].Quality = Items[i].Quality - 1; - } - } + IncreaseQuality(item); } - else + } + else if (item.Name == "Backstage passes to a TAFKAL80ETC concert") + { + IncreaseQuality(item); + if (item.SellIn < 11) { - 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; - } - } - } - } + IncreaseQuality(item); } - - if (Items[i].Name != "Sulfuras, Hand of Ragnaros") + if (item.SellIn < 6) { - Items[i].SellIn = Items[i].SellIn - 1; + IncreaseQuality(item); } - - if (Items[i].SellIn < 0) + if (item.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; - } - } + item.Quality = 0; } } + else if (item.Name == "Sulfuras, Hand of Ragnaros") + { + // Do nothing, the item's quality and sell-in never change. + } + else + { + DecreaseQuality(item); + if (item.SellIn < 0) + { + DecreaseQuality(item); + } + } + } + + private void IncreaseQuality(Item item) + { + if (item.Quality < 50) + { + item.Quality++; + } + } + + private void DecreaseQuality(Item item) + { + if (item.Quality > 0) + { + item.Quality--; + } + } + + private void UpdateItemSellIn(Item item) + { + if (item.Name != "Sulfuras, Hand of Ragnaros") + { + item.SellIn--; + } } } }