[Unity] forループ内でUniRxのストリームを生成したときのバグ

UnityでUniRxを使用していて、forループ内でうまくストリームを生成したときにバグに遭遇しました。
バグの修正方法はわかったのですが、なぜこのようなバグが起きたのか、またもしコードの描き方がよくないならより良い方法を知りたいです。

バグの内容

入力は0から128までの整数を引数にしてinputProvider.GetVelocity(noteNumber)でReactiveProperty<float>とれるようにしています。
以下のコードでは、入力を見て値が0.1f以上だったらコンソールに出力するストリームをforループ内で生成しています。

C#
1            for (var noteNumber = 0; noteNumber < 128; noteNumber++)
2            {
3                _inputProvider.GetVelocity(noteNumber)
4                              .Where(v => v >= 0.1f)
5                              .Subscribe(v =>
6                              {
7                                  Debug.Log($"note:{noteNumber}, velocity:{v}");
8                              });
9            }

しかしこれを実行するとどんな入力(どんな引数=noteNumberからの入力)でも、ログには以下のようになってしまいます。

note:128, velocity:入力の値

そこで以下のようにコードを変更すると今度はうまくいきました。

C#
1            for (var noteNumber = 0; noteNumber < 128; noteNumber++)
2            {
3                // 別の変数に代入して使う。
4                var noteSt = noteNumber.ToString();
5
6                _inputProvider.GetVelocity(noteNumber)
7                              .Where(v => v >= 0.1f)
8                              .Subscribe(v =>
9                              {
10                                  Debug.Log($"note:{noteSt}, velocity:{v}");
11                              });
12            }

これでわかったのはどうやらnoteNumberをそのまま使うとnoteNumberがループの最後の値=128になってそれが使われてしまうということですが、これがどうして起こってしまったかがよくわからないです。

また、この書き方がもしバッドプラクティスであるならよりよい書き方が知りたいです。

よろしくおねがいします。

行動規範の内容に同意します

回答1件

ベストアンサー

おそらく妥当な修正方法じゃないかと思います。
古い記事ですみませんが、「第5回　匿名メソッドとデリゲート（2/4） - ＠IT」の文章を一部引用しますと...

　匿名メソッドを使うプログラマーなら、クロージャを知らない人にも関係する重要な話なので、実際に見てみよう。リスト6は、変数iに0と1の数字を格納しつつ匿名メソッドを作成するコードを2つ含んでいる。1つはfor文、もう1つはForEachメソッドでループを回しているという点が異なる。
C#
1using System;
2
3delegate int SampleMethodDelegate();
4
5class Program
6{
7  static void Main(string[] args)
8  {
9    SampleMethodDelegate[] methods = new SampleMethodDelegate[2];
10
11    // シンプルなforループ
12    for (int i = 0; i < 2; i++)
13    {
14      methods[i] = delegate() { return i; };
15    }
16    Console.WriteLine("{0} {1}", methods[0](), methods[1]());
17    // 出力：2 2
18
19    // ForEachメソッドを使う
20    int[] array = { 0, 1 };
21    Array.ForEach(array, delegate(int i)
22    {
23      methods[i] = delegate() { return i; };
24    });
25    Console.WriteLine("{0} {1}", methods[0](), methods[1]());
26    // 出力：0 1
27  }
28}
リスト6 同じ結果を示さない匿名メソッドの例

　見てのとおり、結果は同じにならない。

　for文でループした方は、どちらのメソッドを呼び出しても2という結果しか得られない。しかし、ForEachメソッドでループした方は0と1という匿名メソッド作成時の値を覚えていて、それを出力してくれる。
だが、これを見て「ForEachメソッドって賢いんですね！」と思うのは早計である。なぜかといえば、ほんの少し書き直すだけ、for文によるループでも同じ結果が得られるためだ。
C#
1using System;
2
3delegate int SampleMethodDelegate();
4
5class Program
6{
7  static void Main(string[] args)
8  {
9    SampleMethodDelegate[] methods = new SampleMethodDelegate[2];
10
11    // シンプルなforループ
12    for (int i = 0; i < 2; i++)
13    {
14      int j = i;
15      methods[i] = delegate() { return j; };
16    }
17    Console.WriteLine("{0} {1}", methods[0](), methods[1]());
18    // 出力：0 1
19  }
20}
リスト7 forループでForEachメソッドと同じ結果を得るコード

　見てのとおり、デリゲート生成時の値を覚えていて、0と1を出力している。果たして、何が違うのだろうか？

　リスト6のforループでは、変数iを使っているが、リスト7では一度、変数jにコピーしてからそれを利用している。この差は一見意味がないかのように見えるかもしれない。だが、キャプチャ機能から見ると、その差は大アリなのだ。
まず、変数iについて見てみよう。この変数はfor文の実行が開始される際に1つだけ作られる。その結果、この1つの変数は2つの匿名メソッドからキャプチャされる。2つの匿名メソッドが読み書きする変数iは同じものである。それ故に、2つの匿名メソッドは同じ値を返す。

　一方、変数jは違う。これはループ内部のスコープに入った時点で生成される変数である。それ故に、2回のループを行えば、2個の別個の変数jが作り出される。2つの匿名メソッドからキャプチャされる変数jは、それぞれ別個のものである。それ故に、2つの匿名メソッドが同じ値を返すとは限らない。