LinqとToList()の動作について

Question

以下の処理をおこなう為、以下のコードを作成しました。
・日付ごとにDepositの合計を求める（ただし、Idが1と2の場合は除外）
・Idが1と2以外の場合、Idを0にする

public class Transaction
{
    public int Id { get; set; }

    public DateTime Date { get; set; }

    public int Deposit { get; set; }
}

public static void Run()
{
    var transactions = new List<Transaction>
    {
        new Transaction { Id = 1, Date = new DateTime(2017, 1, 1), Deposit = 11 },
        new Transaction { Id = 2, Date = new DateTime(2017, 2, 1), Deposit = 12 },
        new Transaction { Id = 3, Date = new DateTime(2017, 3, 1), Deposit = 13 },
        new Transaction { Id = 4, Date = new DateTime(2017, 4, 1), Deposit = 14 },
        new Transaction { Id = 5, Date = new DateTime(2017, 1, 1), Deposit = 15 },
        new Transaction { Id = 6, Date = new DateTime(2017, 2, 1), Deposit = 16 },
        new Transaction { Id = 7, Date = new DateTime(2017, 3, 1), Deposit = 17 },
        new Transaction { Id = 8, Date = new DateTime(2017, 4, 1), Deposit = 18 }
    };

    var result = BundleTransactions(transactions);

    DebugPrintTransactions(result);
    Console.WriteLine(" ");
    DebugPrintTransactions(result.ToList());
}

public static IEnumerable<Transaction> BundleTransactions(IEnumerable<Transaction> transactions)
{
    var notBundleLoanIds = new List<int> { 1, 2 };
    var groupByDate = transactions.GroupBy(a => a.Date).OrderBy(a => a.Key);
    var bundleTransaction = new Transaction { Id = 0 };

    foreach (var group in groupByDate)
    {
        bundleTransaction.Date = group.Key;
        bundleTransaction.Deposit = 0;

        foreach (var transaction in group)
        {
            if (notBundleLoanIds.Any(a => a == transaction.Id))
                yield return transaction;
            else
                bundleTransaction.Deposit += transaction.Deposit;
        }
        yield return bundleTransaction;
    }
}

public static void DebugPrintTransactions(IEnumerable<Transaction> transactions)
{
    foreach (var transaction in transactions)
    {
        Console.WriteLine($"ID:{transaction.Id}, 日付:{transaction.Date:d}, 入金:{transaction.Deposit}");
    }
}

DebugPrintTransactions(result);では以下の結果になります。
（期待どおりの結果です）

ID:1, 日付:2017/01/01, 入金:11
ID:0, 日付:2017/01/01, 入金:15
ID:2, 日付:2017/02/01, 入金:12
ID:0, 日付:2017/02/01, 入金:16
ID:0, 日付:2017/03/01, 入金:30
ID:0, 日付:2017/04/01, 入金:32

しかし、DebugPrintTransactions(result.ToList());では以下の結果になります。
なぜ、こうなるのでしょうか？

ID:1, 日付:2017/01/01, 入金:11
ID:0, 日付:2017/04/01, 入金:32
ID:2, 日付:2017/02/01, 入金:12
ID:0, 日付:2017/04/01, 入金:32
ID:0, 日付:2017/04/01, 入金:32
ID:0, 日付:2017/04/01, 入金:32

Answer 1

原因は他の方がおっしゃるとおりと思いますが、少しだけ一般化した言い方をすれば「LINQ(パイプライン機能)にからむ計算が副作用を持っているから」という表現がよく使われると思います。最初に列挙した値が後続の要素の列挙の間に「変化してしまう」ので。

一般的な対策（というか原則）は「LINQで用いる論理では極力副作用を持たない演算に限定する」ことだと思います。

haru666さんが対処として挙げておられる方法と本質的には差がないのですが、bundleTransactionのような自前での繰り返し制御を書く代わりに、LINQに用意されている単純機能の組み合わせに置き換える書き方もあると思います。

int[] notBundleLoanIds = { 1, 2 };
Func<int, int> groupingId = (x) => notBundleLoanIds.Any(id => id == x) ? x : 0;
var result = transactions
  // 1,2以外のIdを0にマッピングした上で日付とIdでグルーピング
  .GroupBy(t => {
    var Id = groupingId(t.Id);
    return new { t.Date, Id };
  })
  // Id=0についてだけ集約
  .SelectMany(g => {
    if (g.Key.Id == 0) {
      var total = g.Sum(t => t.Deposit);
      return new Transaction[] {
        new Transaction { Id = g.Key.Id, Date = g.Key.Date, Deposit = total }
      };
    } else {
      // Id=1 or 2で同一日付のレコードが複数あるという前提でSelectManyを使っています。
      // 複数ないならSelectに書き直してもよいと思います。
      return g as IEnumerable<Transaction>;
    }
  });
  // この例ではソートしていない

こうした書き方では累積などの計算を自分で書かなくて済むので副作用のある処理が自然に出てこなくなる気がします。

Comments

[haru853]

ありがとうございました。
恥ずかしながら、私のレベルでは最初コードを見ても何をしているのかさっぱりわかりませんでした。
Func<>やLinqの中でのreturnなど知らない使い方でしたので、とても勉強になりました。

[KSwordOfHaste]

そうでしたか。逆に自分はyield returnの方が凝った書き方に見えましたｗ；

Answer 2

イテレータによる評価タイミングとクラスが参照型であることとToListメソッドによるものが原因ではないでしょうか。
下記のようにやれば上手くいくと思います。

public static IEnumerable<Transaction> BundleTransactions(IEnumerable<Transaction> transactions)
{
    var notBundleLoanIds = new List<int> { 1, 2 };
    var groupByDate = transactions.GroupBy(a => a.Date).OrderBy(a => a.Key);

    foreach (var group in groupByDate)
    {
        int deposit = 0;

        foreach (var transaction in group)
        {
            if (notBundleLoanIds.Any(a => a == transaction.Id))
                yield return transaction;
            else
                deposit += transaction.Deposit;
        }
        yield return new Transaction { Id = 0, Date=group.Key, Deposit = deposit };
    }
}

Comments

[haru853]

ありがとうございました。newで返してしまえば良かったのですね。

Answer 3

ToListにより一旦処理されているかされていないかの差です。

BundleTransactionsでは内部で1つのTransactionインスタンスを使いまわしていますね。
直接DebugPrintTransactionsに値を渡した場合はループの途中途中で出力がされるため問題になりませんが、ToList()を使うとyield return bundleTransactionで戻された値は全て同一インスタンスですから、その値は全て同じになってしまいます。

インスタンスの識別はIDがあるので簡単ですね。
ID:1
ID:0
ID:2
ID:0
ID:0
ID:0

この通りにListにはインスタンスが設定されているので、リストを逐次出力すると
bundleTransaction(ID:0)の最終状態が何度も出力されることになってしまいます。

#追記

解決策を書いていませんでした。
ToList()が必要な場合は、bundleTransactionを返す時にクローンを作って戻すことです。
これには2通りの方法があります。

１．クローンメソッドをTransactionクラスに作成して使う
今回はシャローコピーで十分ですから、MemberwiseCloneメソッドが使えます。
シャローコピーで十分な条件というのは中身のデータが全てイミュータブルであるということです。

public class Transaction
{
    public int Id { get; set; }

    public DateTime Date { get; set; }

    public int Deposit { get; set; }

    /// <summary>
    /// 同一の情報を持つ新しい Transaction のインスタンスを生成して返します。
    /// </summary>
    public Transaction CreateClone()
    {
        return (Transaction)MemberwiseClone();
    }
}

後はyield return bundleTransactionをyield return bundleTransaction.CreateClone()に書き換えましょう。

２．Transactionをクラスではなく構造体にしてしまう
16バイト以下ですし、後からデータの修正が起きないのであれば構造体にしてしまうという方法もあります。
単純にclassをstructに変えるだけです。
方法としては単純ですが、構造体の適切な取り扱いが分からない場合は1番を使った方が良いです。

Comments

[haru853]

ありがとうございました。理解できました。
MemberwiseCloneメソッドの事も知らなかった為、勉強になりました。