Working with List<T> with builder pattern - c#

Question

Apologies for the title as I could not quite word my issue, so here we go.

I have a DiscountBuilder class that has two methods WithBuyOneGetOneFree and WithDiscount. They both take a List<ShoppingItem> and work well in isolation like this:

var items = BasketStore.ShoppingItems;

var updatedItems = new DiscountBuilder()
    .WithBuyOneGetOneFree(items)
    .Create();
    
var updatedItems2 = new DiscountBuilder()
    .WithDiscount(75, items)
    .Create();

But when I attempt to chain the both methods to apply WithBuyOneGetOneFree and WithDiscount I only get the list of shopping items returned with WithBuyOneGetOneFree which is completely logical and understandable:

var updatedItems3 = new DiscountBuilder()
    .WithDiscount(75, items)
    .WithBuyOneGetOneFree(items)
    .Create();

What is the best way/design to achieve this so all three examples above all work in complete harmony, so I have the option to use the methods individual like now, and chain the methods together to get both discounts?

Here is the rest of the code.

DiscountBuilder:

#region Private Fields
private List<ShoppingItem> internalShoppingItems;
#endregion

#region Public Methods
public List<ShoppingItem> Create()
{
    return internalShoppingItems;
}

public IDiscountBuilder WithBuyOneGetOneFree(List<ShoppingItem> shoppingItems)
{
    internalShoppingItems = new List<ShoppingItem>();

    foreach(var item in shoppingItems)
    {
        var updateItem = new ShoppingItem
        {
            Id = item.Id,
            Price = item.Price,
            Name = item.Name,
            Quantity = item.Quantity + 1
        };

        internalShoppingItems.Add(updateItem);
    }
    return this;
}

public IDiscountBuilder WithDiscount(int percentage, List<ShoppingItem> shoppingItems)
{
    internalShoppingItems = new List<ShoppingItem>();

    foreach (var item in shoppingItems)
    {
        var updateItem = new ShoppingItem
        {
            Id = item.Id,
            Price = GetDiscountedPrice(percentage, item.Price),
            Name = item.Name,
            Quantity = item.Quantity
        };

        internalShoppingItems.Add(updateItem);
    }
    return this;
}
#endregion

ShoppingItem:

public class ShoppingItem
{
    public int Id { get; set; }
    public double Price { get; set; }
    public string Name { get; set; }
    public int Quantity { get; set; }
}

BasketStore (just where the original dummy data is coming from):

public static class BasketStore
{
    #region Properties
    public static List<ShoppingItem> ShoppingItems { get; private set; }
    #endregion

    #region Constructor
    static BasketStore()
    {
        ShoppingItems = new List<ShoppingItem>()
        {
            new ShoppingItem {
                Id = 1,
                Price = 10.0,
                Name = "IWatch",
                Quantity = 1
            },
            new ShoppingItem {
                Id = 5,
                Price = 9.99,
                Name = "Ladies Watch",
                Quantity = 3
            },
            new ShoppingItem {
                Id = 7,
                Price = 1.75,
                Name = "Ladies Replacement Strap",
                Quantity = 1
            }
        };
    }
    #endregion
}

Answer 1

This might be a classic XY problem. The problem is that each fluent action is overriding the internal list, so last applied discount wins and changes the list.

In terms of best way/design, there are too many possible approaches to this problem.

Here is one simple example.

Change the approach of storing the internal list and overriding it.

public interface IDiscountBuilder {
    IDiscountBuilder WithBuyOneGetOneFree();
    IDiscountBuilder WithDiscount(int percentage);
    List<ShoppingItem> Create(List<ShoppingItem> shoppingItems);
}

Instead store the actions to be applied to the items in the list and then create the list based on the provided input.

For example

public class DiscountBuilder : IDiscountBuilder {
    #region Private Fields        
    private Action<ShoppingItem> buyOneGetOneFree;
    private Action<ShoppingItem> withDiscount;
    #endregion

    #region Public Methods
    public List<ShoppingItem> Create(List<ShoppingItem> shoppingItems) {
        List<ShoppingItem> result = new List<ShoppingItem>();
        foreach (ShoppingItem item in shoppingItems) {
            //copy item details
            ShoppingItem updateItem = new ShoppingItem() {
                Id = item.Id,
                Price = item.Price,
                Name = item.Name,
                Quantity = item.Quantity
            };
            //apply actions
            buyOneGetOneFree?.Invoke(updateItem);
            withDiscount?.Invoke(updateItem);
            result.Add(updateItem);
        }
        return result;
    }

    public IDiscountBuilder WithBuyOneGetOneFree() {
        buyOneGetOneFree = item => {
            item.Quantity = item.Quantity + 1;
        };
        return this;
    }

    public IDiscountBuilder WithDiscount(int percentage) {
        withDiscount = item => {
            item.Price = GetDiscountedPrice(percentage, item.Price);
        };
        return this;
    }

    private double GetDiscountedPrice(int percentage, double price) {
        return price - (price * (percentage / 100D));
    }
    #endregion
}

All 3 examples in the original question can be applied independent of each other

var items = BasketStore.ShoppingItems;

var updatedItems = new DiscountBuilder()
    .WithBuyOneGetOneFree()
    .Create(items);

var updatedItems2 = new DiscountBuilder()
    .WithDiscount(75)
    .Create(items);

var updatedItems3 = new DiscountBuilder()
    .WithDiscount(75)
    .WithBuyOneGetOneFree()
    .Create(items);

This approach can be expanded even further and multiple ways but that would outside of the scope of the original question.