How do I fix this bug: Cant remove object from a list?

Admin 1
Viewed 84

I have this really weird bug where I cannot seem to delete an object from a list. I am using an Injector class to pass the same object(thus the same list) around into multiple forms. The Delete() function is just a generic delete, really nothing special. However in my unit test it keeps failing and I'm unsure why.

Injector class:

    [Serializable]
public class InjectorClass
{
    public OrderAdministration Administration { get; private set; }
    public WareHouse WareHouse { get; private set; } 

    public InjectorClass()
    {
        Administration = new OrderAdministration(this);
        WareHouse= new WareHouse();
    }
}

Constructor of the WareHouse class:

public List<Part> Catalogus { get; private set; }
    public  List<Part> PartsInStock { get; private set; }

    public WareHouse()
    {
        PartsInStock = new List<Part>();
        AddStandardStockToCatalogus();
        AddStockPartsToStock();
    }

    private void AddStandardStockToCatalogus()
    {
        Body body1 = new Body("Body7zk", 1, "Ebony", "Blue");
        Body body2 = new Body("SuperXtrem368", 2, "Maple", "Red");
        Neck neck1 = new Neck("Heavy-Slider", 3, "Juniper", true);
        Neck neck2 = new Neck("SlickFingerBoard", 4, "Oak", false);
        Fretboard fretboard1 = new Fretboard("PrettySlamFinger", 5, "Oak", false);
        Fretboard fretboard2 = new Fretboard("GentleToucher", 6, "Ebony", true);
        Pickups pickups1 = new Pickups("TubeScreamers", 7, Pickups.PickupType.P90);
        Pickups pickups2 = new Pickups("BluesBrothers", 8, Pickups.PickupType.Humbucker);
        Bridge bridge1 = new Bridge("MetalGearRipper", 9, false);
        Bridge bridge2 = new Bridge("BridgeOfLove", 10, true);

        Catalogus = new List<Part> { body1, body2, neck1, neck2, fretboard1, fretboard2, pickups1, pickups2, bridge1, bridge2};
    }

    private void AddStockPartsToStock()
    {
        foreach(Part p in Catalogus)
        {
            PartsInStock.Add(p);
        }
    }

The Delete() function in the same Warehouse class:

public int GetStock(Part neededPart)
    {
        if(neededPart == null)
        {
            throw new ArgumentNullException();
        }
        int numberInStock = 0;
        foreach(Part p in PartsInStock)
        {
            if(p.Id == neededPart.Id)
            {
                numberInStock++;
            }
        }

        return numberInStock;
    }
    public bool AddToStock(Part arrivedPart)
    {
        if(arrivedPart == null)
        {
            throw new ArgumentNullException();
        }
        if(FindByName(arrivedPart.Name).Name == arrivedPart.Name)
        {
            PartsInStock.Add(arrivedPart);
            return true;
        }
            

        return false;
    }

    public bool RemovePart(Part usedPart)
    {
        if (GetStock(usedPart) >= 1)
        {
            PartsInStock.Remove(usedPart);
            return true;
        }

        return false;
    }

The unit test I run:

Neck neck1 = new Neck("Heavy-Slider", 3, "Juniper", true);
        InjectorClass injector = new InjectorClass();
        Assert.AreEqual(10, injector.WareHouse.PartsInStock.Count);
        injector.WareHouse.RemovePart(neck1);
        Assert.AreEqual(9, injector.WareHouse.PartsInStock.Count);

The output on both the assert functions is 10

Edit: I have re-written my search and delete function to the following:

Search function:

public Part FindByNameStock(Part neededPart)
    {
        if (neededPart == null)
        {
            throw new ArgumentNullException();
        }
        foreach (Part p in PartsInStock)
        {
            if (p.Name == neededPart.Name)
            {
                return p;
            }
        }

        return null;
    }

Delete function:

public bool RemovePart(Part usedPart)
    {
        Part part = FindByNameStock(usedPart);
        if (part != null)
        {
            PartsInStock.Remove(part);
            return true;
        }

        return false;
    }

The code now works and the unit test too.

object list unit-testing debugging c#
3 Answers

This line PartsInStock.Remove(usedPart); will try to remove the following object

new Neck("Heavy-Slider", 3, "Juniper", true);

which is not in your list. The reason is that reference type equality is not based on the properties, but on the actual reference. Whenever you run new a new reference is created.

var neck1 = new Neck("Heavy-Slider", 3, "Juniper", true);
var neck2 = new Neck("Heavy-Slider", 3, "Juniper", true);
Console.WriteLine(neck1 == neck2); //This is false

Check the code here for an example: https://dotnetfiddle.net/7gAWID

Check here for the IEquatable implementation that will make your code work.

user_16436760

GetStock uses the Id of the part to find it, whereas PartsInStock.Remove tries to remove the part by reference - and the reference you have is different to the item in the collection you're trying to remove from.

public bool RemovePart(Part usedPart)
{
    if (GetStock(usedPart) >= 1) // item is found here
    {
        PartsInStock.Remove(usedPart); // but usedPart is a different reference - nothing is removed
        return true;
    }

    return false;
}
user_2196610

Two separate objects of Neck although having the same ID are different objects and hence the item is never removed. Moreover, you can return the value of List.Remove as it also returns a bool if an item was actually removed. So if no item was removed, it returns false:

public bool RemovePart(Part usedPart) 
=>        GetStock(usedPart) >= 1 ?
            PartsInStock.Remove(usedPart);
            : false;

Use RemoveAll() to remove the item by ID. RemoveAll() returns an int representing the number of items removed:

public bool RemovePart(Part usedPart)
{
    if (GetStock(usedPart) >= 1)
    {
        PartsInStock.RemoveAll(part => part.Id == usedPart.Id);
        return true;
    }

    return false;
}

Also, since your RemovePart returns a bool. You can add a check to ensure if it was removed.

if (injector.WareHouse.RemovePart(neck1)) 
{
    Assert.AreEqual(9, injector.WareHouse.PartsInStock.Count);
}
user_114551050
Related