F＃ - 健全性チェックとオプション

Question

私はF＃の新機能ですので、何年ものC＃/ Java OOPの後で私の考え方を変えるのは難しいです。

私はイベントハンドラMyForm.SelectFile(filePath:String)を使用してダイアログを開き、読み込むファイルを選択できるようにしました。ファイルを選択すると、Parser.LoadFile(filePath:String)が呼び出されます：「アルファ」と「ベータ」：

static member LoadFile(filePath:String) = 
    if not <| ZipFile.IsZipFile(filePath) then 
     failwith "invalid file specified." 
    use zipFile = new ZipFile(filePath) 
    if zipFile.Count <> 2 || zipFile |> Seq.exists(fun x -> x.FileName <> "alpha" && x.FileName <> "beta") then 
     failwith "invalid file specified." 
    zipFile |> fun x -> Parser.Parse(x.OpenReader()) 

私は常に拡張子なしの2つのファイルを含む有効なzipアーカイブであることを選択したファイルを期待しています。

まず、自分の入力を消毒するには良い方法がありますか？

私のif文はかなり長く、私はF＃がよりよい解決策を提供できると確信していますが、本当にわかりません。

第2に、failwithを使用すると、私のMyForm.SelectFile(filePath:String)メソッドで例外を処理することが強制され、オプションがより良い解決策になると思います。

私がZipFileをインスタンス化する必要があるため、2つの異なる連続したチェック（ZipFile.IsZipFileと内容）を実行する必要がある場合、それらを使用する方法を理解できません。

C＃では、チェックが失敗した場合にnullを返すだけで、戻り値をnullと照合すると、エラーを表示するか続行する必要があるかどうかがわかります。

現在のコード：それはのように書かれていた場合

type Parser with 

    static member isValidZipFile (zipFile:ZipFile) = 
     (zipFile.Count = 2) && (zipFile |> Seq.forall(fun x -> (x.FileName = "alpha") || (x.FileName = "beta"))) 

    static member LoadFile(filePath:String) = 
     if not <| ZipFile.IsZipFile(filePath) then 
      None 
     else 
      use zipFile = new ZipFile(filePath) 
      if not <| Parser.isValidZipFile(zipFile) then 
       None 
      else 
       Some(seq { for zipEntry in zipFile do yield Parser.Parse(zipEntry.OpenReader()) } |> Seq.toArray) 

Answer 1

まず、あなたの関数の最後の行は、もう少しエレガント可能性：

zipFile.OpenReader() |> Parser.Parse 

第二に、あなたは正しい軌道に乗っていますあなたが考えている限り、Optionを使用しています。それは、この場合には、実際にはかなり簡単です：

static member LoadFile(filePath:String) = 
    if not <| ZipFile.IsZipFile(filePath) then None else 
    use zipFile = new ZipFile(filePath) 
    if zipFile.Count <> 2 || zipFile |> Seq.exists(fun x -> x.FileName <> "alpha" && x.FileName <> "beta") then None else 
    Some (zipFile.OpenReader() |> Parser.Parse) 

最後の行は、のように書くこともできること：

今

zipFile.OpenReader() |> Parser.Parse |> Some 

、あなたが長いif文は好きではないと述べました。それを関数に変換しましょう！そして、私は通常、「正の」名前の関数を好んでいます。すなわち、isValidInput関数は通常isInvalidInputよりも役に立ちます。それでは、zipファイルが実際に有効であるかどうかを確認する機能を作成してみましょう：

let isValid (z:ZipFile) = 
    z.Count = 2 && z |> Seq.forAll(fun x -> x.FileName = "alpha" || x.FileName = "beta") 

今すぐあなたのLoadFile機能になることができます。

static member LoadFile(filePath:String) = 
    if not <| ZipFile.IsZipFile(filePath) then None else 
    use zipFile = new ZipFile(filePath) 
    if not <| isValid zipFile then None else 
    zipFile.OpenReader() |> Parser.Parse |> Some 

そして、それは読み非常に簡単になりますので、私たちは今のリファクタリング停止することができます。

Answer 2

このコードは、のように見えます。です。このようなシンプルなコードにシーケンス式を使用することは過度のものです。

Some(seq { for zipEntry in zipFile do yield Parser.Parse(zipEntry.OpenReader()) } |> Seq.toArray) 

あなたはこの

zipFile |> Seq.map (fun ze -> ze.OpenReader() |> Parser.parse) |> Some 

のように良いことを書くことができそれとも、（なぜ？）の配列でそれを行う際に主張すれば

zipFile |> Seq.map (fun ze -> ze.OpenReader() |> Parser.parse) |> Seq.toArray |> Some 

あなたは型シグネチャになってしまいますoption<seq<value>>。私はこれが良い考えであるかどうかはわかりませんが、コードの残りの部分を見ずに言うことはできません。