Apostrofix
Apostrofix

Reputation: 2180

How to update all duplicates from List<Objects> if they exist in another list?

I am trying to update all duplicates in a list of objects if they exist in another collection.

This is the code that I have:

public class Cars
{
    public string brand { get; set; }
    public string model { get; set; }
    public string allowed { get; set; }
}

var notAllowedModels = GetNotAllowedModels(); // returns a List<String> with model names

foreach(var car in cars) // cars is a List<Car>
{
    if (notAllowedModels.Contains(car.model))
       cars[cars.FindIndex(x => x.model == car.model)].allowed = "No";
    else
       cars[cars.FindIndex(x => x.model == car.model)].allowed = "Yes";
}

That works fine if the models in the list are unique, but if they exist more than once, then only the first one will be updated, while the others will be null.

Can anybody think of a way to update all duplicates if they exist in the notAllowedModels list?

I know I can use FindAll instead of FindIndex but it returns a list and I am not sure exactly how it helps me, since it will be the same issue with the duplicates.

Upvotes: 0

Views: 67

Answers (4)

Alberto Monteiro
Alberto Monteiro

Reputation: 6229

You can use GroupBy to group cars by model, then check if the model is allowed, if its not, then you loop all cars in the grouped Model and set allowed to false. vice-versa to allowed cars. Like this code:

public class Cars
{
    public string brand { get; set; }
    public string model { get; set; }
    public string allowed { get; set; }
}

var notAllowedModels = new HasSet<string>(GetNotAllowedModels()); // using hasset to improve the performance

foreach(var carGroup in cars.GroupBy(car => car.model)) // cars is a List<Car>
{
    if (notAllowedModels.Contains(carGroup.Key))
        foreach(var c in carGroup)
            c.allowed = "No";
    else
        foreach(var c in carGroup)
            c.allowed = "Yes";
}

Obs.: You should use HasSet in notAllowedModels, to improve the performance of your code, since Dictionary is indexed, and List doesnt, and you are loop in the notAllowedCars everytime in your loop by car.

Upvotes: 0

Roy T.
Roy T.

Reputation: 9638

You can also us the Linq Union extension method to find all cars in the cars list that are also in the notAllowedModels list.

var union = cars.Union(notAllowedModels);
union.ForEach(x => x.allowed = "No");

The Union method uses the default comparerer. So you should overload the Equals method of your cars class.

On a side note, its often better to use a bool than a string type for values that can either false or true and that its better to use properties than making your members public.

public class Car
{
    public bool IsAllowed {get;set;}
    // rest of your class
}

Upvotes: 1

Tim Schmelter
Tim Schmelter

Reputation: 460208

You don't need to use List.FindIndex to find the car, you have it already:

foreach(Car car in cars) // cars is a List<Car>
{
    if (notAllowedModels.Contains(car.model))
       car.allowed = "No";
    else
       car.allowed = "Yes";
}

By the way, if you want to improve performance return a HashSet<string> from GetNotAllowedModels. It only contains unique values and is very efficient in lookups(Contains).

Upvotes: 6

Murray Foxcroft
Murray Foxcroft

Reputation: 13765

You can refine this further but will something along the lines of the below work?

 foreach (var notAllowedCar in cars.Where((car) => notAllowedModels.Contains(car.model)))
        {
            notAllowedCar.allowed = "False";
        }

Upvotes: 0

Related Questions