Наиболее эффективный способ найти элемент по определенному индексу в List <>, изменить элемент и затем обновить List <>?

Question

Взять List<Car>.Каждый Car имеет уникальный индекс, идентифицирующий его, скажем, RegNumber, а затем другое свойство, описывающее его - например, Color.

Я хочу

1. проверить, еслиВ коллекции есть автомобиль с RegNumber 5
2. , если он есть, измените цвет
3. , если нет, добавьте новый элемент для этого автомобиля
4. сохраните список

Это то, как я сейчас это делаю, и я спрашиваю, есть ли лучший, более эффективный способ сделать это?

Car car = CarsForSale.Find(c => c.RegNumber == 5);

if (car != null)
{
   foreach (Car car in CarsForSale)
   {
      if (car.RegNumber == 5)
      {
         car.Color = "Red";
         break;
      }
   }
}
else
{
   CarsForSale.Add(new Car(5, "Red"));
}

Save(CarsForSale);

РЕДАКТИРОВАТЬ Не существует нескольких автомобилей с одинаковым регистром - RegNumber уникален, как указано в вопросе.

Это был действительно просто тупой момент в любом случае, который мог бы заметить обзор кода.Спасибо за все ответы и за то, что не дразнили мой явно тупой вопрос.Конечно, элемент / элемент, возвращаемый из коллекции, является ссылкой, так что абсолютно не нужно повторять список снова ... по-моему, время биться головой об стену.

Answer 1

Ну, во-первых, вам не нужен тест для car.RegNumber == 5 в цикле - вы уже нашли первую машину, которая соответствует этому критерию из вашего утверждения:

Car car = CarsForSale.Find(c => c.RegNumber == 5);

На самом деле весь ваш цикл избыточен, вы можете просто иметь:

if (car != null)
{
    car.Color = "Red";
}
else
{
    CarsForSale.Add(new Car(5, "Red"));
}

Если вы не хотите найти все автомобили, у которых RegNumber равно 5, в этом случае ваша первая строка будет неправильной, поскольку будет найдена только первая машина, которая соответствует критериям. Чтобы найти все автомобили, которые вы хотите что-то по этим направлениям:

var cars = CarsForSale.Where(c => c.RegNumber == 5);

foreach (Car car in cars)
{
    car.Color = "Red";
}

if (!car.Any())
{
    CarsForSale.Add(new Car(5, "Red"));
}

С вашим исходным кодом компилятор должен был предупредить вас, что переопределение car в цикле скроет оригинальное определение (которое я цитировал).

Answer 2

Обновление

В ответ на этот комментарий вы оставили еще пару ответов:

Что делать, если есть несколько машин с RegNumber 5?

Если несколько машин могут иметь одинаковый RegNumber, то вызов Find - неправильный подход. Find просто перечисляет список, чтобы найти совпадение; вам будет лучше пропустить это и сохранить свой цикл foreach.

Однако вы можете сделать ваш код более кратким, используя вместо этого Where:

var matches = CarsForSale.Where(c => c.RegNumber == 5);
int numMatches = 0;

foreach (Car match in matches )
{
    match.Color = "Red";
    ++numMatches;
}
if (numMatches == 0)
{
   CarsForSale.Add(new Car(5, "Red"));
}

---

Оригинальный ответ

Весь этот цикл foreach является избыточным: вы в основном выполняете ту же работу, которую уже выполняли, вызывая Find.

Так что код можно упростить:

Car car = CarsForSale.Find(c => c.RegNumber == 5);

if (car != null)
{
    car.Color = "Red";
}
else
{
   CarsForSale.Add(new Car(5, "Red"));
}

Тем не менее, если вы ищете автомобили в вашем List<Car> на RegNumber, то имеет смысл использовать Dictionary<int, Car> вместо List<Car>:

Car car;
if (CarsForSale.TryGetValue(5, out car))
{
    car.Color = "Red";
}
else
{
    CarsForSale[5] = car = new Car(5, "Red");
}

Answer 3

Если у вас есть машина, которую вы ищете из оператора LINQ, нет необходимости возвращаться к коллекции, чтобы найти совпадение:

Car car = CarsForSale.Where(c => c.RegNumber == 5).FirstOrDefault();

if(car != null)
{
    car.Color = "Red";
}
else
{
    CarsForSale.Add(new Car(5, "Red"));
}

Save(CarsForSale);

Или, если будет несколько автомобилей с одинаковым номером RegNumber:

var cars = CarsForSale.Where(c => c.RegNumber == 5);

if(cars.Any())
{
    foreach(Car car in cars)
        car.Color = "Red";
}
else
{
    CarsForSale.Add(new Car(5, "Red"));
}

Save(CarsForSale);

Answer 4

Почему вы повторяете список, если у вас уже есть результат?

Это достигнет того же результата:

Car car = CarsForSale.Find(c => c.RegNumber == 5);
if (car != null)
{
   car.Color = "Red";
}
else
{
   CarsForSale.Add(new Car(5, "Red"));
}
Save(CarsForSale);

Результат метода Find для CarsForSale, если он возвращает результат, будет ссылочным типом, что означает, что любые изменения в car также изменят элемент в CarsForSale. Полагаю, вы подумали, что результат из Find будет отключен от фактического элемента в CarsForSale, отсюда и ненужный цикл foreach?

Answer 5

Итак, как уже упоминал Дэн, если у вас есть уникальное свойство, вы должны использовать его в качестве ключа в Dictionary<TKey, TValue>.

.O (1) операция, в то время как в Списке это только O (n) в худшем случае (и теперь представьте, что у вас есть 1 миллион автомобилей в вашем списке).

var carsForSale = new Dictionary<int, Car>();

//Create a car which you like to check
var checkCar = new Car(4, Color.Red);

//Use this approach if you want to change only a few properties
//of an existing item
if (carsForSale.ContainsKey(checkCar.RegNum))
{
    carsForSale[checkCar.RegNum].Color = checkCar.Color;
}
else
{
    carsForSale[4] = checkCar;
}

//If you have to take over ALL property settings, you can also
//forget the old item and take the new one.
//The index operator is smart enough to just add a new one
//or to delete an old and add the new in one step.
carsForSale[checkCar.RegNum] = checkCar;

Фиктивная реализация класса автомобиля:

public class Car
{
    public int RegNum { get; private set; }
    public Color Color { get; set; }

    public Car(int regNum)
        : this(regNum, Color.Empty)
    { }

    public Car(int regNum, Color color)
    {
        RegNum = regNum;
        Color = color;
    }
}

Проблема , почему вы используете словарь, заключается в том, что вы хотите явно сказать, что это за ключ (свойство RegNum вашего автомобиля), но вы также можете использоватьHashset<T>, если ваш объект Car правильно реализует Equals() и GetHashCode(), но это немного сложнее, чем вы думаете.Хорошее объяснение можно найти в книге Essentials C # .