Могу ли я переписать этот цикл do-while как цикл foreach?

Question

Есть ли способ написать этот код более элегантно с помощью цикла foreach? Логика «создать новую запись» мешает мне, потому что она должна выполняться, даже если pendingEntries не содержит элементов.

ItemDto itemToAdd; // an input parameter to the method
IEnumerator<Item> pendingEntries = existingPendingItems.GetEnumerator();
pendingEntries.MoveNext();
do // foreach entry
{
  Item entry = pendingEntries.Current;
  if (entry != null) // fold the itemToAdd into the existing entry
  {
    entry.Quantity += itemToAdd.Quantity; // amongst other things
  }
  else // create a new entry
  {
    entry = Mapper.Map<ItemDto, Item>(itemToAdd);
  }
  Save(entry);
} while (pendingEntries.MoveNext());

Answer 1

Это должно быть переписано.Я не знаю, какую коллекцию вы используете, но Current в вашем случае не определено, поскольку MoveNext могло бы вернуть false.Как указано в документации :

Ток не определен при любом из следующих условий: Последний вызов MoveNext вернул false, что указывает на окончание сбора.

Вот как я бы переписал его:

bool isEmpty = true;
foreach (Item entry in existingPendingItems)
{
  ProcessEntry(entry, itemToAdd);
  isEmpty = false;
}
if (isEmpty)
{
  ProcessEntry(null, itemToAdd);
}

• ProcessEntry содержит логику для одной записи и легко проверяется модулем.
• Алгоритм очищен для чтения.
• Перечисляемое значение по-прежнему перечисляется только один раз.

Answer 2

foreach (Item entry in existingPendingItems.DefaultIfEmpty())
{
    Item entryToSave;

    if (entry != null) // fold the itemToAdd into the existing entry
    {
        entry.Quantity += itemToAdd.Quantity; // amongst other things

        entryToSave = entry;
    }
    else // create a new entry
    {
        entryToSave = Mapper.Map<ItemDto, Item>(itemToAdd);
    }

    Save(entryToSave);
}

Ключ - это вызов Enumerable.DefaultIfEmpty () - он вернет последовательность с элементом default (Item), если последовательность пуста. Это будет null для ссылочного типа.

Редактировать: исправлена ​​ошибка, упомянутая neotapir.

Answer 3

Лично я бы предложил что-то вроде этого:

ItemDto itemToAdd; // an input parameter to the method
if (existingPendingItems.Any())
{
    foreach(Item entry in existingPendingItems)
    {
        entry.Quantity += itemToAdd.Quantity    
        Save(entry);
    }
}
else
{
    entry = Mapper.Map<ItemDto, Item>(itemToAdd);
    Save(entry);
}

Я думаю, что это делает смысл кода намного более понятным.

РЕДАКТИРОВАТЬ: Изменено количество для любого согласно предложениюТакже исправлена ​​логика добавления количества

Answer 4

var pendingEntries = existingPendingItems.Any()
    ? existingPendingItems
    : new List<Item> { Mapper.Map<ItemDto, Item>(itemToAdd) };

foreach (var entry in pendingEntries)
{
    entry.Quantity += itemToAdd.Quantity; // amongst other things
    Save(entry);
}

Идея в том, что вы настраиваете себя на успех перед повторением.Что вы собираетесь повторить?Либо существующие записи, если таковые имеются, либо просто новая запись в противном случае.

Обрабатывая это заранее, вы знаете, что у вас есть что-то, с чем можно работать, ваша петля остается очень чистой.

Answer 5

foreach( Item entry in pendingEntries.Current)
{
    if( entry != null)
        entry.Quantity += itemToAdd.Quantity;
    else
       entry = Mapper.Map<ItemDto, Item>(itemToAdd);
    Save(entry)
}

не могу точно проверить это без предметов

Answer 6

Я бы переписал его как более стандартный while метод.И вы забыли, что IEnumerator<T> реализует IDisposable, поэтому вы должны избавиться от него.