Skip to content

Instantly share code, notes, and snippets.

@davidwhitney
Created August 24, 2024 18:14
Show Gist options
  • Save davidwhitney/bb0d611d50ade87abdc5e09137294378 to your computer and use it in GitHub Desktop.
Save davidwhitney/bb0d611d50ade87abdc5e09137294378 to your computer and use it in GitHub Desktop.
Claude AI C# Guilded Rose Test

Prompt: Make this code not suck:

using System.Collections.Generic;
namespace GildedRoseKata;
public class GildedRose
{
    IList<Item> Items;
    public GildedRose(IList<Item> Items)
    {
        this.Items = Items;
    }
    public void UpdateQuality()
    {
        for (var i = 0; i < Items.Count; i++)
        {
            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;
                    }
                }
            }
        }
    }
}

Here's a refactored version of the code that aims to improve readability, maintainability, and adherence to best practices:

using System.Collections.Generic;

namespace GildedRoseKata
{
    public class GildedRose
    {
        private const string AgedBrie = "Aged Brie";
        private const string BackstagePasses = "Backstage passes to a TAFKAL80ETC concert";
        private const string Sulfuras = "Sulfuras, Hand of Ragnaros";
        private const int MaxQuality = 50;
        private const int MinQuality = 0;

        private readonly IList<Item> _items;

        public GildedRose(IList<Item> items)
        {
            _items = items;
        }

        public void UpdateQuality()
        {
            foreach (var item in _items)
            {
                UpdateItemQuality(item);
            }
        }

        private void UpdateItemQuality(Item item)
        {
            if (item.Name == Sulfuras)
            {
                return; // Sulfuras never changes
            }

            DecreaseSellIn(item);

            switch (item.Name)
            {
                case AgedBrie:
                    UpdateAgedBrie(item);
                    break;
                case BackstagePasses:
                    UpdateBackstagePasses(item);
                    break;
                default:
                    UpdateStandardItem(item);
                    break;
            }
        }

        private void DecreaseSellIn(Item item)
        {
            item.SellIn--;
        }

        private void UpdateAgedBrie(Item item)
        {
            IncreaseQuality(item);
            if (item.SellIn < 0)
            {
                IncreaseQuality(item);
            }
        }

        private void UpdateBackstagePasses(Item item)
        {
            IncreaseQuality(item);

            if (item.SellIn < 10)
            {
                IncreaseQuality(item);
            }

            if (item.SellIn < 5)
            {
                IncreaseQuality(item);
            }

            if (item.SellIn < 0)
            {
                item.Quality = MinQuality;
            }
        }

        private void UpdateStandardItem(Item item)
        {
            DecreaseQuality(item);
            if (item.SellIn < 0)
            {
                DecreaseQuality(item);
            }
        }

        private void IncreaseQuality(Item item)
        {
            if (item.Quality < MaxQuality)
            {
                item.Quality++;
            }
        }

        private void DecreaseQuality(Item item)
        {
            if (item.Quality > MinQuality)
            {
                item.Quality--;
            }
        }
    }
}

Would you like me to explain or break down the changes made to the code?

Prompt: make it more extensible

Certainly. To make the code more extensible, we can implement the Strategy pattern and introduce interfaces for different item types. This approach will make it easier to add new item types in the future without modifying existing code. Here's a more extensible version:

using System.Collections.Generic;

namespace GildedRoseKata
{
    public class GildedRose
    {
        private readonly IList<Item> _items;
        private readonly Dictionary<string, IItemStrategy> _strategies;

        public GildedRose(IList<Item> items)
        {
            _items = items;
            _strategies = new Dictionary<string, IItemStrategy>
            {
                { "Aged Brie", new AgedBrieStrategy() },
                { "Backstage passes to a TAFKAL80ETC concert", new BackstagePassesStrategy() },
                { "Sulfuras, Hand of Ragnaros", new SulfurasStrategy() }
            };
        }

        public void UpdateQuality()
        {
            foreach (var item in _items)
            {
                GetStrategy(item).UpdateQuality(item);
            }
        }

        private IItemStrategy GetStrategy(Item item)
        {
            return _strategies.TryGetValue(item.Name, out var strategy) 
                ? strategy 
                : new StandardItemStrategy();
        }
    }

    public interface IItemStrategy
    {
        void UpdateQuality(Item item);
    }

    public abstract class BaseItemStrategy : IItemStrategy
    {
        protected const int MaxQuality = 50;
        protected const int MinQuality = 0;

        public abstract void UpdateQuality(Item item);

        protected void DecreaseSellIn(Item item)
        {
            item.SellIn--;
        }

        protected void IncreaseQuality(Item item)
        {
            if (item.Quality < MaxQuality)
            {
                item.Quality++;
            }
        }

        protected void DecreaseQuality(Item item)
        {
            if (item.Quality > MinQuality)
            {
                item.Quality--;
            }
        }
    }

    public class StandardItemStrategy : BaseItemStrategy
    {
        public override void UpdateQuality(Item item)
        {
            DecreaseSellIn(item);
            DecreaseQuality(item);

            if (item.SellIn < 0)
            {
                DecreaseQuality(item);
            }
        }
    }

    public class AgedBrieStrategy : BaseItemStrategy
    {
        public override void UpdateQuality(Item item)
        {
            DecreaseSellIn(item);
            IncreaseQuality(item);

            if (item.SellIn < 0)
            {
                IncreaseQuality(item);
            }
        }
    }

    public class BackstagePassesStrategy : BaseItemStrategy
    {
        public override void UpdateQuality(Item item)
        {
            DecreaseSellIn(item);
            IncreaseQuality(item);

            if (item.SellIn < 10)
            {
                IncreaseQuality(item);
            }

            if (item.SellIn < 5)
            {
                IncreaseQuality(item);
            }

            if (item.SellIn < 0)
            {
                item.Quality = MinQuality;
            }
        }
    }

    public class SulfurasStrategy : BaseItemStrategy
    {
        public override void UpdateQuality(Item item)
        {
            // Sulfuras never changes
        }
    }
}

Would you like me to explain the changes and how this design improves extensibility?

Prompt: name the strategies less specific so they're not referring to specific items, but the characteristics of those items

Certainly. Let's rename the strategies to focus on the characteristics of the items rather than their specific names. This approach will make the code even more flexible and reusable. Here's the updated version:

using System.Collections.Generic;

namespace GildedRoseKata
{
    public class GildedRose
    {
        private readonly IList<Item> _items;
        private readonly Dictionary<string, IItemStrategy> _strategies;

        public GildedRose(IList<Item> items)
        {
            _items = items;
            _strategies = new Dictionary<string, IItemStrategy>
            {
                { "Aged Brie", new AppreciatingItemStrategy() },
                { "Backstage passes to a TAFKAL80ETC concert", new EventItemStrategy() },
                { "Sulfuras, Hand of Ragnaros", new LegendaryItemStrategy() }
            };
        }

        public void UpdateQuality()
        {
            foreach (var item in _items)
            {
                GetStrategy(item).UpdateQuality(item);
            }
        }

        private IItemStrategy GetStrategy(Item item)
        {
            return _strategies.TryGetValue(item.Name, out var strategy) 
                ? strategy 
                : new DepreciatingItemStrategy();
        }
    }

    public interface IItemStrategy
    {
        void UpdateQuality(Item item);
    }

    public abstract class BaseItemStrategy : IItemStrategy
    {
        protected const int MaxQuality = 50;
        protected const int MinQuality = 0;

        public abstract void UpdateQuality(Item item);

        protected void DecreaseSellIn(Item item)
        {
            item.SellIn--;
        }

        protected void IncreaseQuality(Item item)
        {
            if (item.Quality < MaxQuality)
            {
                item.Quality++;
            }
        }

        protected void DecreaseQuality(Item item)
        {
            if (item.Quality > MinQuality)
            {
                item.Quality--;
            }
        }
    }

    public class DepreciatingItemStrategy : BaseItemStrategy
    {
        public override void UpdateQuality(Item item)
        {
            DecreaseSellIn(item);
            DecreaseQuality(item);

            if (item.SellIn < 0)
            {
                DecreaseQuality(item);
            }
        }
    }

    public class AppreciatingItemStrategy : BaseItemStrategy
    {
        public override void UpdateQuality(Item item)
        {
            DecreaseSellIn(item);
            IncreaseQuality(item);

            if (item.SellIn < 0)
            {
                IncreaseQuality(item);
            }
        }
    }

    public class EventItemStrategy : BaseItemStrategy
    {
        public override void UpdateQuality(Item item)
        {
            DecreaseSellIn(item);
            IncreaseQuality(item);

            if (item.SellIn < 10)
            {
                IncreaseQuality(item);
            }

            if (item.SellIn < 5)
            {
                IncreaseQuality(item);
            }

            if (item.SellIn < 0)
            {
                item.Quality = MinQuality;
            }
        }
    }

    public class LegendaryItemStrategy : BaseItemStrategy
    {
        public override void UpdateQuality(Item item)
        {
            // Legendary items never change
        }
    }
}

In this version, we've renamed the strategies to reflect the characteristics of the items:

StandardItemStrategy → DepreciatingItemStrategy AgedBrieStrategy → AppreciatingItemStrategy BackstagePassesStrategy → EventItemStrategy SulfurasStrategy → LegendaryItemStrategy

These new names focus on the behavior of the items rather than their specific names, making the strategies more reusable and the system more flexible. For example, if a new item is introduced that appreciates in value over time, we can simply use the AppreciatingItemStrategy for it without creating a new strategy.

Would you like me to explain how this naming convention improves the extensibility and flexibility of the system?

Prompt: have those strategies try to match against the items as they iterate through, rather than having an explicit mapping from known items

Certainly. This approach will make the system even more flexible and extensible. We'll modify the code to have each strategy determine if it can handle a given item, rather than relying on an explicit mapping. Here's the updated version:

using System.Collections.Generic;
using System.Linq;

namespace GildedRoseKata
{
    public class GildedRose
    {
        private readonly IList<Item> _items;
        private readonly IEnumerable<IItemStrategy> _strategies;

        public GildedRose(IList<Item> items)
        {
            _items = items;
            _strategies = new List<IItemStrategy>
            {
                new LegendaryItemStrategy(),
                new EventItemStrategy(),
                new AppreciatingItemStrategy(),
                new DepreciatingItemStrategy() // This should be last as it's the default
            };
        }

        public void UpdateQuality()
        {
            foreach (var item in _items)
            {
                var strategy = _strategies.First(s => s.CanHandle(item));
                strategy.UpdateQuality(item);
            }
        }
    }

    public interface IItemStrategy
    {
        bool CanHandle(Item item);
        void UpdateQuality(Item item);
    }

    public abstract class BaseItemStrategy : IItemStrategy
    {
        protected const int MaxQuality = 50;
        protected const int MinQuality = 0;

        public abstract bool CanHandle(Item item);
        public abstract void UpdateQuality(Item item);

        protected void DecreaseSellIn(Item item)
        {
            item.SellIn--;
        }

        protected void IncreaseQuality(Item item)
        {
            if (item.Quality < MaxQuality)
            {
                item.Quality++;
            }
        }

        protected void DecreaseQuality(Item item)
        {
            if (item.Quality > MinQuality)
            {
                item.Quality--;
            }
        }
    }

    public class DepreciatingItemStrategy : BaseItemStrategy
    {
        public override bool CanHandle(Item item) => true; // Default strategy

        public override void UpdateQuality(Item item)
        {
            DecreaseSellIn(item);
            DecreaseQuality(item);

            if (item.SellIn < 0)
            {
                DecreaseQuality(item);
            }
        }
    }

    public class AppreciatingItemStrategy : BaseItemStrategy
    {
        public override bool CanHandle(Item item) => item.Name == "Aged Brie";

        public override void UpdateQuality(Item item)
        {
            DecreaseSellIn(item);
            IncreaseQuality(item);

            if (item.SellIn < 0)
            {
                IncreaseQuality(item);
            }
        }
    }

    public class EventItemStrategy : BaseItemStrategy
    {
        public override bool CanHandle(Item item) => item.Name == "Backstage passes to a TAFKAL80ETC concert";

        public override void UpdateQuality(Item item)
        {
            DecreaseSellIn(item);
            IncreaseQuality(item);

            if (item.SellIn < 10)
            {
                IncreaseQuality(item);
            }

            if (item.SellIn < 5)
            {
                IncreaseQuality(item);
            }

            if (item.SellIn < 0)
            {
                item.Quality = MinQuality;
            }
        }
    }

    public class LegendaryItemStrategy : BaseItemStrategy
    {
        public override bool CanHandle(Item item) => item.Name == "Sulfuras, Hand of Ragnaros";

        public override void UpdateQuality(Item item)
        {
            // Legendary items never change
        }
    }
}

Key changes in this version:

We've added a CanHandle method to the IItemStrategy interface and its implementations. The GildedRose class now maintains a list of strategies instead of a dictionary. In the UpdateQuality method, we use First to find the first strategy that can handle the item. Each strategy now determines if it can handle an item based on the item's characteristics. The DepreciatingItemStrategy acts as a default strategy by always returning true for CanHandle.

This approach allows for more flexibility:

New item types can be added by creating new strategies without modifying existing code. The order of strategies in the list determines their priority. Strategies can use more complex logic to determine if they can handle an item, not just based on the name.

Would you like me to explain any part of this implementation in more detail?

Prompt: Instead of the strategies checking against one single string, convert them all to check against an internal list of items that match their criteria.

Excellent suggestion. This approach will make the strategies even more flexible and easier to extend. Here's the updated version:

using System.Collections.Generic;
using System.Linq;

namespace GildedRoseKata
{
    public class GildedRose
    {
        private readonly IList<Item> _items;
        private readonly IEnumerable<IItemStrategy> _strategies;

        public GildedRose(IList<Item> items)
        {
            _items = items;
            _strategies = new List<IItemStrategy>
            {
                new LegendaryItemStrategy(),
                new EventItemStrategy(),
                new AppreciatingItemStrategy(),
                new DepreciatingItemStrategy() // This should be last as it's the default
            };
        }

        public void UpdateQuality()
        {
            foreach (var item in _items)
            {
                var strategy = _strategies.First(s => s.CanHandle(item));
                strategy.UpdateQuality(item);
            }
        }
    }

    public interface IItemStrategy
    {
        bool CanHandle(Item item);
        void UpdateQuality(Item item);
    }

    public abstract class BaseItemStrategy : IItemStrategy
    {
        protected const int MaxQuality = 50;
        protected const int MinQuality = 0;
        protected readonly HashSet<string> ApplicableItems;

        protected BaseItemStrategy(IEnumerable<string> applicableItems)
        {
            ApplicableItems = new HashSet<string>(applicableItems);
        }

        public virtual bool CanHandle(Item item) => ApplicableItems.Contains(item.Name);
        public abstract void UpdateQuality(Item item);

        protected void DecreaseSellIn(Item item)
        {
            item.SellIn--;
        }

        protected void IncreaseQuality(Item item)
        {
            if (item.Quality < MaxQuality)
            {
                item.Quality++;
            }
        }

        protected void DecreaseQuality(Item item)
        {
            if (item.Quality > MinQuality)
            {
                item.Quality--;
            }
        }
    }

    public class DepreciatingItemStrategy : BaseItemStrategy
    {
        public DepreciatingItemStrategy() : base(new string[] { }) { }

        public override bool CanHandle(Item item) => true; // Default strategy

        public override void UpdateQuality(Item item)
        {
            DecreaseSellIn(item);
            DecreaseQuality(item);

            if (item.SellIn < 0)
            {
                DecreaseQuality(item);
            }
        }
    }

    public class AppreciatingItemStrategy : BaseItemStrategy
    {
        public AppreciatingItemStrategy() : base(new[] { "Aged Brie" }) { }

        public override void UpdateQuality(Item item)
        {
            DecreaseSellIn(item);
            IncreaseQuality(item);

            if (item.SellIn < 0)
            {
                IncreaseQuality(item);
            }
        }
    }

    public class EventItemStrategy : BaseItemStrategy
    {
        public EventItemStrategy() : base(new[] { "Backstage passes to a TAFKAL80ETC concert" }) { }

        public override void UpdateQuality(Item item)
        {
            DecreaseSellIn(item);
            IncreaseQuality(item);

            if (item.SellIn < 10)
            {
                IncreaseQuality(item);
            }

            if (item.SellIn < 5)
            {
                IncreaseQuality(item);
            }

            if (item.SellIn < 0)
            {
                item.Quality = MinQuality;
            }
        }
    }

    public class LegendaryItemStrategy : BaseItemStrategy
    {
        public LegendaryItemStrategy() : base(new[] { "Sulfuras, Hand of Ragnaros" }) { }

        public override void UpdateQuality(Item item)
        {
            // Legendary items never change
        }
    }
}

Key changes in this version:

The BaseItemStrategy now has a protected ApplicableItems HashSet and a constructor that initializes it. The CanHandle method in BaseItemStrategy checks if the item's name is in the ApplicableItems set. Each strategy subclass now calls the base constructor with a list of applicable item names. The DepreciatingItemStrategy still overrides CanHandle to always return true, making it the default strategy.

This approach offers several benefits:

It's easy to add new items to a strategy by adding their names to the list in the constructor. Strategies can handle multiple items with similar behavior. It's more efficient to check against a HashSet than using string comparison. The code is more maintainable and extensible.

For example, if we wanted to add a new appreciating item, we could simply modify the AppreciatingItemStrategy constructor:

public AppreciatingItemStrategy() : base(new[] { "Aged Brie", "Fine Wine" }) { }

This would make the strategy handle both "Aged Brie" and "Fine Wine" without any other code changes. Would you like me to explain any part of this implementation in more detail?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment