Task中のループについて

前提・実現したいこと

2つのフォルダーがあってその中のcsvファイルを取得し、
そのcsvファイルに対して処理をしたいです。
タスクを利用している理由としてはこれらの処理を
フォルダーごとに並列処理がしたいのです。

発生している問題・エラーメッセージ

9行目のTask.Run()に入ったところでエラーになります。
デバッグで確認するとi=2(folders.Length)になっていて
for文ではi<folders.Lengthなので0と1にしかならないと思ってます。

エラーメッセージ
IndexOutOfRangeException インデックスが配列の境界外です

該当のソースコード

C#
ソースコード
string[] folders = new string[2], files = new string[100];
private Task Run_Task()
{
    var tasks = new List<Task>();
    for (int i = 0; i < folders.Length; i++)
    {
        var task = Task.Run(() => 
        {
            files = Directory.GetFiles(folders[i], "*.csv");
            //取得したcsvファイルに対する処理
        });
        tasks.Add(task);
    }
    return Task.WhenAll(tasks);
}

試したこと

補足情報（FW/ツールのバージョンなど）

VisualStudio2017

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

回答4件

ベストアンサー

こんにちは。
既に回答が付いているので、今回問題になったキャプチャの仕組みについて説明してみます。

まず答えを書いてしまうと、以下のやり方で解決します。

csharp
1for (int i = 0; i < folders.Length; i++)
2{
3    var x = i; // for文中のスコープ内に変数を確保
4    var task = Task.Run(() => 
5    {
6        // files変数は内部で宣言すること！
7        var files = Directory.GetFiles(folders[x], "*.csv"); // xを使用
8        //取得したcsvファイルに対する処理
9    });
10    tasks.Add(task);
11}

ラムダ式による外部変数のキャプチャは、その変数のスコープによって共有範囲が定まるので、for文の1繰り返し内にのみ生存する変数をキャプチャすることで、必ず正しいindexの値がラムダ式内で利用できるようになります。iはfor文によって書き換えられていますが、xはコード内で書き換えられていないからです。

既にYamakawaJunichiさんが回答でサジェストしていますが、このコードではforeachを使ったほうが圧倒的に楽です。

csharp
1foreach (var folder in folders)
2{
3    var task = Task.Run(() => 
4    {
5        var files = Directory.GetFiles(folder, "*.csv");
6        //取得したcsvファイルに対する処理
7    });
8    tasks.Add(task);
9}

処理を行うのに不要なiとかi++とかfolders.Lengthとかのノイズをコードから消し去ることができます。

投稿2018/02/07 03:54

tamoto

総合スコア4103

yaneuranoneko

2018/02/08 01:00

回答ありがとうございます。 foreachだと確かに可読性も上がる気がします。変数はfor文内やTask.Runの中で定義するというのは気が付かなかったので助かりました！ありがとうございます。

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

Task内の処理とfor文を回す処理が別スレッドなので、i++してi < folders.Lengthを判定することと、files = Directory.GetFiles(folders[i], "*.csv")を実行することの順序は保障されません。

よって、i=1のとき
・[メインスレッド]Task.Runでワーカースレッドを立てる
・[メインスレッド]i++し、iが2になる
・[メインスレッド]i < folders.Lengthを判定しfalseなのでループを抜ける
・[ワーカースレッド]files = Directory.GetFiles(folders[i], "*.csv")を実行する

となるとfolders[2]にアクセスしてしまいます。

for文をforeach文にするのが良いと思います。

追記
files = Directory.GetFiles(folders[i], "*.csv");ですが、同じfilesに複数回結果を代入しているので、複数回Task.Runしても最後に完了したデータしか取れません。
string[][]やList<string[]>に入れるなどすべきだと思います。

投稿2018/02/05 07:39

編集2018/02/05 07:59

YamakawaJunichi

総合スコア630

yaneuranoneko

2018/02/05 08:09

ありがとうございます。 foreach文は試してみます！ filesのところは間違えてました。

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

こんにちは。

その振る舞いからすると、i の記憶領域は１つしかなく、Run_Task()関数、および、起動した全てのタスクで共有されているのでは？
各タスクを起動しても実行が直ぐに始まるわけではありませんから、その間にRun_Task()関数で i の値が2に更新されているのだろうと思います。

私自身はC#のラムダ式でパラメータを使ったことはないですが、可能な筈です。
int型は値型ですからiをパラメータで渡せば、その時点でコピーされますので意図通りに動作するだろうと思います。

投稿2018/02/05 07:27

編集2018/02/05 07:28

Chironian

総合スコア23272

yaneuranoneko

2018/02/05 07:56

var task = Task.Run(() => { int n = i; files = Directory.GetFiles(folders[n], "*.csv"); //取得したcsvファイルに対する処理 }); ここの3行目のように追加するという意味でよろしいですか？

Chironian

2018/02/05 08:06

いえ、それではタスクの実行が始まってからのコピーになるので間に合わない可能性があります。 https://www.ipentec.com/document/csharp-give-parameter-to-task-using-state ↑のproc関数をパラメータ付きラムダ式に置き換えるイメージでいけるだろうと思います。