Reputation: 2180
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
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
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
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
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