Paul Almond
Paul Almond

Reputation: 127

For Loop Is Not running for the full loop duration

I have a set of loops that adds or removes to and from a collection list when a quantity is updated. The removal loop works perfectly however the addition loop only applies a shortened amount of times.

Here are the loops

public async Task UpdateLineItemByOrderLineId(int orderLineId, int newQuantity)
    {
        var clientBasket = await GetBasketAsync();
        var lineItem = clientBasket.OrderLines.FirstOrDefault(x => x.OrderLineId == orderLineId);
        if (newQuantity == 0)
        {
            foreach (var m in lineItem.DelegatesList.Where(f=>f.OrderLineId == orderLineId))
            {
                lineItem.DelegatesList.Remove(m);
            }
            _orderLinesRepository.Delete(lineItem);
            _orderLinesRepository.Save();
        }
        else
        {
            lineItem.Quantity = newQuantity;

            if (lineItem.DelegatesList.Count > newQuantity)
            {
                for (int i = lineItem.DelegatesList.Count - 1; i >= newQuantity; --i)
                {
                    lineItem.DelegatesList.RemoveAt(i);
                }
            }
            if (lineItem.DelegatesList.Count < newQuantity)
            {
                for (int z = 0; z <= newQuantity - lineItem.DelegatesList.Count; z++)
                {
                    lineItem.DelegatesList.Add(new OrderDelegate());
                }
            }
            await _basketRepository.SaveAsync();
        }
    }

On sample data the following happens:
If newQuantity = 8 and delegateslists.count = 1 it only runs 4 times (taking delegateslist.count to 5)

I've tripled checked the math but cannot see why its not running for the full amount of times.

Upvotes: 2

Views: 100

Answers (2)

Sudsy1002
Sudsy1002

Reputation: 598

for (int z = 0; z <= newQuantity - lineItem.DelegatesList.Count; z++)
{
    lineItem.DelegatesList.Add(new OrderDelegate());
}

Your for loop includes newQuantity - lineItem.DelegatesList.Count.

The lineItem.DelegatesList.Count is increasing with each iteration of the loop.

This means that as Z increases, it is comparing to a number that is decreasing.

I.E. First run, z = 0, newQuantity = 8, lineItem.DelegatesList.Count = 1.

Second run, z = 1, newQuantity = 8, lineItem.DelegatesList.Count = 2.

Third run, z = 2, newQuantity = 8, lineItem.DelegatesList.Count = 3.

Fourth run, z = 3, newQuantity = 8, lineItem.DelegatesList.Count = 4.

Fifth run, z = 4, newQuantity = 8, lineItem.DelegatesList.Count = 5.

 z <= newQuantity - lineItem.DelegatesList.Count;

on the fifth run, 4 <= 8-5 (3).

You could just take the initial count and work with that instead.

else
{
    lineItem.Quantity = newQuantity;
    int initialCount = lineItem.DelegatesList.Count;

    if (lineItem.DelegatesList.Count > newQuantity)
    {
        for (int i = lineItem.DelegatesList.Count - 1; i >= newQuantity; --i)
        {
                lineItem.DelegatesList.RemoveAt(i);
        }
    }
    if (lineItem.DelegatesList.Count < newQuantity)
    {
        for (int z = 0; z <= newQuantity - initialCount; z++)
        {
            lineItem.DelegatesList.Add(new OrderDelegate());
        }
    }
    await _basketRepository.SaveAsync();
}

Upvotes: 3

RecyclingBen
RecyclingBen

Reputation: 310

When you add to lineItem.DelegatesList you increase lineItems.DelegatesList.Count. This means that it will only run about half as many times as it should.

Either store the count in a temporary variable prior to starting the loop, or add the items into a separate list, adding the list into line Item.DelegatesList afterwards.

Upvotes: 0

Related Questions