usingでCSVを開いたらToArrayして返す[CsvHelper]

# この記事は何か

自分で書いた過去の記事の中で腑に落ちない箇所があるので
追加で検証し、コードを改善するための記事

対象の記事：

https://qiita.com/t13801206/items/852f1bcccaf618a3a346

[結論](#結論)にコードの変更前後を記載

# 追記 20201/09/20

@albireo様からコメントいただき、さらにコードを改善したので[付録](#付録)に掲載する

# 問題のコード

自分で書いていて、なんか違和感を感じたコード
[対象の記事](https://qiita.com/t13801206/items/852f1bcccaf618a3a346#%E3%83%A2%E3%83%87%E3%83%AB)から一部を抜粋して掲載する

```cs:CSVファイルのレコードを読み込んで返すメソッド
public static IEnumerable<CSVTable> ReadCsv(string csvPath)
{
    // CSVファイルを読み込んで、レコードをテーブルにして返すメソッド
    //  --- 中略 ---

    // 読み込んだデータをモデルに詰めて、メソッドの戻り値とする
    return csv.GetRecords<CSVTable>()
        .Select(x => new CSVTable() // <--- 検証２　Select でプロパティ代入をやめる
        {
            Month = x.Month,
            CreateDate = x.CreateDate,
            FiscalYear = x.FiscalYear,
            Title = x.Title,
            Price = x.Price,
        }).ToList(); // <--- 検証１　これを削除してみる
}
```

```cs:メソッドを呼び出す側
// CSVを読み込んでデータベースに格納する
var tabel = Models.CSVTable.ReadCsv();
foreach (var record in tabel)
    context.Add(record);
```

## 違和感・検証したいこと

- 戻り値が`IEnumerable`なのに、最後`ToList()`してるのはなぜ？
    - メモリ節約を考えて、`IEnumerable`のまま返したほうがいいのではないか？
        - ＜検証１＞**`ToList()`を削除して、`IEnumerable`のまま返してみる**
- `CSVTable`インスタンスを生成するときに`Id`プロパティを代入してないけど、**結局デフォルト値を使っているのでは？**
    - デフォルト値は固定値だから、**SQL Server の主キーとして重複するのでは？**
        - ＜検証２＞**`.Select()`でプロパティを詰め替える作業をやめてみる** 

# ＜検証１＞IEnumerable型で返してみる

レコードを返すメソッドの戻り値が`IEnumerable`なので（なのでと言いつつ、そうしたのは自分）
メソッドの`return`時に`.ToList()`をやめて、`.Select()`したままを返してみる

```diff_c_sharp:CSVファイルのレコードを読み込んで返すメソッド
 public static IEnumerable<CSVTable> ReadCsv(string csvPath)
 {
     return csv.GetRecords<CSVTable>()
         .Select(x => new CSVTable()
         {  
-        }).ToList(); // <--- 検証１　これを削除してみる
+        });
 }
```

`.Select()`メソッドの戻り値は`IEnumerable<T>`型のコレクションになる

## ＜検証１＞の結果：`.ToList()`を削除したら、エラーが出た

> System.ObjectDisposedException: 'GetRecords<T>() returns an IEnumerable<T> that yields records. This means that the method isn't actually called until you try and access the values. e.g. .ToList() Did you create CsvReader inside a using block and are now trying to access the records outside of that using block?
ObjectDisposed_ObjectName_Name'

↓↓↓ Google翻訳 ↓↓↓

> System.ObjectDisposedException： 'GetRecords <T>（）は、レコードを生成するIEnumerable <T>を返します。これは、値にアクセスしようとするまで、メソッドが実際に呼び出されないことを意味します。例えば.ToList（）usingブロック内にCsvReaderを作成し、そのusingブロック外のレコードにアクセスしようとしていますか？
ObjectDisposed_ObjectName_Name '

エラー文を完璧に理解できていないが、`using`句には身に覚えがある・・・
ローカルの CSV ファイルをストリームで読みだすので、`using`で破棄するようにしている

```cs:CSVファイルのレコードを読み込んで返すメソッドの序盤
using CsvHelper.CsvReader csv = new(stream, cultureInfo);
```

### ＜検証１＞の考察・まとめ

おそらく以下の理由で、戻り値は`.ToList()`で実体化していないといけない

- `using`で変数 csv を宣言している
    - `using` で csv の生存期間を限定して、ファイルを開きっぱなしにしないようにしている
- IEnumerable が返ったとき、つまりメソッド最下部の`return`を抜けたときには、すでに csv は破棄されている（たぶん）
    - `yield return`されたものは、それを使うときにアクセスするけど、そのときに csv はもういない・・・
        - エラーです

#### ＜こんなときは？＞巨大CSVを扱うから`yield return`したい

どうしようもなくCSVが巨大なときは、コレクションを返すのではなく
逆に`DbContext`を受け取って、データベースに格納する作業を代行してあげるのも手かもしれない

# ＜検証２＞`GetRecords<T>()`の後の`.Select()`は不要だった

`.Select()`で`CSVTable`インスタンスを生成するときに
`Id`プロパティへの代入を避けながら`CSVTable`に詰め替えていたけど
**そんなことしなくても、データベースに直接格納することができた**

主キーが衝突するとか心配する必要はなかった

```diff_c_sharp:CSVファイルのレコードを読み込んで返すメソッド
 public static IEnumerable<CSVTable> ReadCsv(string csvPath)
 {
-    return csv.GetRecords<CSVTable>()
-        .Select(x => new CSVTable()
-        {
-            Month = x.Month,
-            CreateDate = x.CreateDate,
-            FiscalYear = x.FiscalYear,
-            Title = x.Title,
-            Price = x.Price,
-        }).ToList();
 
+    return csv.GetRecords<CSVTable>().ToList();
}
```

なんなら`Id = x.Id`としても大丈夫だった

## `ToList()`と`ToArray()`どっちを使うか？

`IEnumerable`で返すので、派生先の`ToList`と`ToArray`のどちらにしても戻り値型としては正しい
参照記事を~~サラッと~~読んだ限り、**どっちでもいいときには`ToArray`を使うといいらしい**

```cs:CSVファイルのレコードを読み込んで返すメソッド
public static IEnumerable<CSVTable> ReadCsv(string csvPath)
{
    // リスト
    return csv.GetRecords<CSVTable>().ToList();
    // 配列
    return csv.GetRecords<CSVTable>().ToArray();
}
```

## 戻り値型は`IEnumerable`でいいのか？

>メソッド内部でリストを生成して返すのなら、素直に List<T> を戻り値にすればよい。無理に IEnumerable<T> にする必要はない（ただし設計上の理由によりそうする場合もある）。
>理由は簡単で、より具体的なインタフェースで返す方が、呼び出し元での利便性が高まるからである。

引用元：[メソッド戻り値の型 - 引数の型を何でも List にしちゃう奴にそろそろ一言いっておくか](https://qiita.com/lobin-z0x50/items/248db6d0629c7abe47dd#%E3%83%A1%E3%82%BD%E3%83%83%E3%83%89%E6%88%BB%E3%82%8A%E5%80%A4%E3%81%AE%E5%9E%8B)

たしかに`ToList()`して`IEnumerable`で返したものを
呼び出し側で再度`ToList()`されるのは無駄でしかない

であれば、最初から使える型で返してあげるとよい
どうせ`ToList()`しているのだから、戻り値型を限定したところで、メソッドの仕事は増えない

# 結論

**変更前**

```cs:CSVファイルのレコードを読み込んで返すメソッド
public static IEnumerable<CSVTable> ReadCsv(string csvPath) // <--- 戻り値はより具体的に配列を返す
{
    return csv.GetRecords<CSVTable>()
        .Select(x => new CSVTable() // <--- Select でプロパティ代入を不要
        {
            Month = x.Month,
            CreateDate = x.CreateDate,
            FiscalYear = x.FiscalYear,
            Title = x.Title,
            Price = x.Price,
        }).ToList(); // <--- ToArray()に変更
}
```

**変更後**

```cs:CSVファイルのレコードを読み込んで返すメソッド
public static CSVTable[] ReadCsv(string csvPath)
{
    return csv.GetRecords<CSVTable>().ToArray();
}
```

# おわりに

動いたところで満足することなく、日々精進しよう（自戒）

## ほかのことに気づいた

検証の過程で何度も SQL Server にデータを入れていて
気づいたら`Id`列が飛び飛びの値になっていた

![image.png](https://qiita-image-store.s3.ap-northeast-1.amazonaws.com/0/272511/b4e88a42-d266-2034-e243-011cc9ab9ea1.png)

条件が重なるとまれに起きるらしい
設定で変更できそう

https://kikki.hatenablog.com/entry/2015/11/23/180637

# 参考

https://qiita.com/hiki_neet_p/items/75bf39838ce580cca92d

https://qiita.com/NCT48/items/d8394d587e9fee969ca9#foreach%E3%81%A3%E3%81%A6%E3%81%A9%E3%81%86%E3%81%AA%E3%81%A3%E3%81%A6%E3%82%8B%E3%81%AE

https://qiita.com/Zuishin/items/36a2cf3e12bbedbfb61e

http://neue.cc/2011/08/03_336.html

# 付録

## モデル

- 作成済みのCsvReaderを返す

```cs:CSVTable.cs(一部)
        //指定したファイルのCsvReaderを作成して返す
        public static CsvHelper.CsvReader OpenCsvReader(string csvPath = @"./data.csv")
        {
            // 1. ファイルをチェックする
            if (!File.Exists(csvPath))
                throw new FileNotFoundException($"File Not Found!! {csvPath}");

            if (!csvPath.EndsWith(".csv", StringComparison.CurrentCultureIgnoreCase))
                throw new FormatException($"Invalid Extension!! {csvPath}");

            // 2. CsvReader を使うための準備
            CultureInfo cultureInfo = new("ja-JP");
            StreamReader stream = new(csvPath, Encoding.UTF8);

            return new(stream, cultureInfo);
        }

        //せっかくなのでジェネリックメソッドにしてみた
        public static IEnumerable<T> ReadCsv<T>(CsvHelper.CsvReader csv)
            => csv.GetRecords<T>();  //SeceltもToArrayも不要なのでメソッドにする必要ないかも…

```

## 使う側

- 使う側で Reader を using して忘れずに破棄する
- Enumerable のまま使えるので、`AddRange`で一括格納できる

```cs:使う側
        public MainWindow()
        {
            InitializeComponent();

            // 1. コンテキスト生成する
            Context.CSVTableContext context = new();

            // 2. CSVを読み込んでデータベースに格納する
            using (var csv = Models.CSVTable.OpenCsvReader())
            {
                // Case1: CSVを読み込んでそのままデータベースに格納する
                //        AddRange()の引数にIEnumerableを与えると、まとめてデータベースに格納できる
                //context.AddRange(Models.CSVTable.ReadCsv<Models.CSVTable>(csv));

                // Case2: CSVの内容をちょっと加工してから格納する
                foreach (var record in Models.CSVTable.ReadCsv<Models.CSVTable>(csv))
                {
                    record.Title += record.Title;
                    context.Add(record);
                }
            }

            // 3. データベースの変更を保存する
            context.SaveChanges();
        }
```

1回ループしたEnumerableのインデックスは末尾まで動いてしまうので、次のループは回らずスルーするので
Case1 と Case2 は、一方をつかうときは、もう一方はコメントアウトしておく