我能否将这个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());

- neontapir

---

所以你可以在名为“existingPendingItems”的列表中拥有不存在（null）的条目？奇怪的命名... - Sjoerd

Save(entry) 是什么意思？它是添加到 existingPendingItems 还是其他列表中？如果在迭代过程中可以更改 existingPendingItems，则不应使用 foreach。 - Sjoerd

@Sjoerd 我认为空值检查的目的仅在于处理枚举器没有任何项的情况。 - Jay

@Jay，你是正确的。null检查是为了处理集合为空的情况。 - neontapir

@neontapir 那你为什么要使用相对罕见的do-while()而不是通常的while()循环呢？当它无效时，你不应该访问Current - 请参阅下面的答案。 - Sjoerd

Answer 1

这段话需要重写。我不知道你使用的是什么类型的集合，但是在 MoveNext 返回 false 的情况下，Current 是未定义的。根据文档所述：

如果满足以下任一条件，则 Current 未定义： 上次调用 MoveNext 返回 false，表示已到达集合的末尾。

以下是我的重写版本：

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

• ProcessEntry 包含单个条目的逻辑，并且易于进行单元测试。
• 该算法易于阅读。
• 可枚举对象仍然只被枚举一次。

- Julien Lebosquain

---

第一次调用 MoveNext() 的问题你抓得很好。一开始没有提到这个。+1。 - Ivan Danilov

这是真的，如果集合中没有任何项，那么当前项就是未定义的（=null）。 - neontapir

未定义并不意味着“null”。未定义可能是任何东西，甚至会抛出异常（例如，这实际上就是数组所做的）。 - Julien Lebosquain

@JulienLebosquain，我在阅读了Current文档后明白了你的观点。看起来如果MoveNext()返回false，它会抛出异常。 - neontapir

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 提到的 bug。

- Paul Ruane

---

这是一种有趣的方法，但它涉及到写入循环变量，而编译器不允许这样做。 - neontapir

@neotapir：我的脑内编译器显然没有C#编译器那么全面。已修复。 - Paul Ruane

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);
}

我认为这可以使代码的意图更清晰。 编辑：根据建议将计数更改为任何。还修复了添加数量逻辑。

- Mykroft

---

这里的问题是你在foreach块中放置的所有逻辑都必须在else块中为该项复制，这使得代码变得脆弱。此外，因为我们只知道existingPendingItems是可枚举的，所以您必须调用.Count()扩展方法来比较0；在这种情况下，最好使用.Any()，因为它会尽快返回true，只要可以确定至少有一个项目，而不是计算所有项目以查看是否有一个。 - Jay

Answer 4

我会将其重写为更标准的 while 方法。而且你忘了 IEnumerator 实现了 IDisposable，所以你应该将其处理掉。

- Ivan Danilov

Answer 5

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

没有这些物品，无法进行准确的测试

- Grant

---

但是，如果 pendingEntries.Current 中没有任何内容，则 foreach 循环将不会执行。OP 希望循环至少执行一次（"创建新条目" 逻辑正在阻挠我，因为它即使在 pendingEntries 不包含任何项目的情况下也需要执行）。 - Dirk

pendingEntries.Current 是一个单独的 Item，无法使用 foreach。 - Jay

可以用if else语句进行封装。 - Grant

Answer 6

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);
}

这里的想法是在迭代之前为自己设定成功的条件。你将要迭代什么？如果有现有条目，那么就迭代现有条目；否则就新建一个条目。

通过提前处理这个问题，你知道自己有可用的东西可以操作，从而使你的循环非常简洁。

- Jay

---

我喜欢你的想法，但是我认为如果我们从itemToAdd创建一个新的pendingEntries集合，这个实现将最终导致数量翻倍。 - neontapir

@neontapir 哦，我明白了——你实际上不想在新项目上进行任何额外的处理，只是保存它；是这样吗？ - Jay

正确，保存操作会向数据库添加新项目。该方法可以防止重复项被添加。 - neontapir