3

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());
4

6 に答える 6

4

これは書き直す必要があります。使用しているコレクションの種類はわかりませんが、が返される可能性がCurrentあるため、この場合は未定義です。ドキュメントに記載されているように：MoveNextfalse

Currentは、次のいずれかの条件下で未定義です。MoveNextへの最後の呼び出しがfalseを返しました。これは、コレクションの終了を示します。

これが私がそれを書き直す方法です：

bool isEmpty = true;
foreach (Item entry in existingPendingItems)
{
  ProcessEntry(entry, itemToAdd);
  isEmpty = false;
}
if (isEmpty)
{
  ProcessEntry(null, itemToAdd);
}
  • ProcessEntry単一エントリのロジックが含まれており、ユニットテストが容易です。
  • アルゴリズムは読み取り用にクリアされています。
  • 列挙可能なものはまだ一度だけ列挙されます。
于 2011-08-03T17:53:33.960 に答える
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によって言及されたバグを修正しました。

于 2011-08-03T17:49:11.780 に答える
1

個人的に私はこのようなものを提案したいと思います：

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

これにより、コードの意図がはるかに明確になると思います。

編集：提案に従ってカウントを任意に変更しました。数量の追加ロジックも修正されました

于 2011-08-03T17:48:31.997 に答える
0

より標準的な方法として書き直しwhileます。IEnumerator<T>そして、あなたはそれを実装するのを忘れたIDisposableので、あなたはそれを処分するべきです。

于 2011-08-03T17:40:42.727 に答える
0 
foreach( Item entry in pendingEntries.Current)
{
    if( entry != null)
        entry.Quantity += itemToAdd.Quantity;
    else
       entry = Mapper.Map<ItemDto, Item>(itemToAdd);
    Save(entry)
}

アイテムなしで正確にテストすることはできません

于 2011-08-03T17:45:26.647 に答える
0 
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);
}

ここでの考え方は、繰り返す前に成功するための準備をすることです。何を繰り返すつもりですか？既存のエントリ（存在する場合）、または新しいエントリ（存在しない場合）。

これを前もって処理することで、作業するものがあることがわかり、ループが非常にクリーンに保たれます。

于 2011-08-03T17:48:32.417 に答える