CODEWITHSUNDEEP
c#performancelist
abhi
abhi

Reputation: 3136

Quicker way to separate lists

I have two classes.

class Vehicle{
    public string VehicleId {get;set;}
    public string VinNo {get;set;}
    public int ModelYear {get;set;}
    public string Make {get;set;}
    public bool WaterDamaged {get;set;}
    }

class Vins{
    public string VinNo {get;set;}
}

I have populated a list of vehicles from the database using this class structure. So the code looks somewhat like this.

List<Vehicle> vehicles = GetAllVehicles();

Another list I have is sourced from an file. This list contains all the VINs that are water damaged. I was able to use the same structure for this class as above.

List<Vins> damaged = ReadFile();

List<Vehicles> damagedGoods = new List <Vehicles>();
List<Vehicles> goodGoods = new List <Vehicles>();

I need to create two separate XML files using this info. The first will be called DamagedVehicles_{date} and the next will be GoodVehicles_{date}.

So what I did was write a loop like this.

foreach(var v in damaged)
{
    foreach(var v2 in vehicles)
    {
        if(v.VinNo == v2.VinNo) 
        {
            damagedGoods.Add(new Vehicle{});
        }
        else
        {
            goodGoods.Add(new Vehicle{});
        }
    }
}

This is causing a minor issue. Firstly the goodGoods are getting duplicates, which I later weed out. Secondly if I receive a list of 80,000 vehicles, it takes a long time for this to process. Is there some way I can speed the processing and avoid the duplicates?

Upvotes: 1

Views: 50

Answers (1)

Servy
Servy

Reputation: 203817

Your nested foreach is performing a cross product of the two lists. That's...not what you want. Not only is it an inherently expensive operation, but it's results simply aren't in line with what you want.

What you want to do is something like this:

foreach(var vehicle in vehicles)
{
    if(damaged.Contains(vehicle.VinN) 
    {
        damagedGoods.Add(new Vehicle{});
    }
    else
    {
        goodGoods.Add(new Vehicle{});
    }
}

(Note the outer loop is removed entirely.)

This can be further improved due to the fact that List is not particularly efficient at searching. If we use a HashSet to hold onto the damaged vehicles the Contains will be much faster. This is easy enough to do:

HashSet<Vins> damaged = new HashSet<Vins>(ReadFile());

Upvotes: 4

Related Questions