2011-12-16 8 views
3

例外処理のベストプラクティスにはいくつか質問があることがわかりましたが、私が見つけたサンプルについていくつか微調整を加えなければなりませんでした。私の目標は、 "ArgumentException"、 "FileNotFoundException"、 "ArrayIndexOutOfBoundsException"などの基本的な例外を取り除くことでした。この例外処理をきれいに書いていますか?

次のコードを階層内に配置することができます。したがって、Process()はCSVData()を呼び出すValidateData()を呼び出します。私は基本的に同じ種類のものを実行しています。は私のカスタム関数を含む既知の関数をすべてラップしてから、そのメソッドの例外に入れて内部の例外として渡します。これはこの仕事をうまくやっていく方法ですか?それは私のために働いていますが、過去の私のプロジェクトが「catch(Exception e)...」と「e.Messageの解析...」でカバーされているので、非常にクリーンな方法で例外を処理する習慣を得たいと思います。 「

ここに私のコード例があります:

public class CSVData 
{ 
    DataTable data; 
    public CSVData(string file) 
    { 
     try 
     { 
      this.data = CSVReader.ToDataTable(file); //throws a few basic exceptions 
     } 
     catch (FileNotFoundException e) 
     { 
      throw new FailedLoadingCSVException(e, currentLine, file); 
     } 
     catch (IOException e) 
     { 
      throw new FailedLoadingCSVException(e, currentLine, file); 
     } 
     catch (ArgumentNullException e) 
     { 
      throw new FailedLoadingCSVException(e, currentLine, file); 
     } 
     catch (ArgumentException e) 
     { 
      throw new FailedLoadingCSVException(e, currentLine, file); 
     } 
     catch (OutOfMemoryException e) 
     { 
      throw new FailedLoadingCSVException(e, currentLine, file); 
     } 
    } 

    class FailedLoadingCSVException : Exception 
    { 
     public int failedAtLine; 

     public FailedLoadingCSVException(Exception e, int failedLine, string file) 
      :base ("The system failed at loading "+file+" at line "+failedLine, e) 
     { 
      this.failedAtLine = failedLine; 
     } 
    } 
} 
+4

本当に問題に関連していないが、 'OutOfMemoryException'をラップしようとしないでください。その例外が発生した場合、アプリケーション全体が不安定な状態になる可能性があるため、実際にデータを損失するリスクよりも、アプリケーションをバブルアップしてクラッシュさせる方が良いでしょう。 – carlosfigueira

+0

新しい例外を投げているのは例外の再マッピングだけなので、例外処理ではありません。どのような非常に頻繁に非常に小さな値である –

+0

なぜ非常に多くの例外..?なぜ基本的な試しキャッチをしないで、例外の使用(例外e)を書くか、e.messageを記録するか、e.InnerExceptionまたはe.TargetSite.ToString()を取得することができます少し過剰のように見える – MethodMan

答えて

3
try 
{ 
} 
catch (Exception ex) 
{ 
    if (e is FileNotFoundException || 
     e is IOException || 
     // enumerate few 
     ) 
     throw new FailedLoadingCSVException(e, currentLine, file); 
    else 
     throw; // rethrow all other exceptions without touching them   
} 
+0

もしそうなら、彼はExceptionでSwitch Caseを実行することもできますが、スタックトレースを行ってもシンプルなExceptionは動作するはずです。元のコードはクリーンなコードのようには見えません。 – MethodMan

+0

これは間違いなくはるかにクリーンですが、コンセプトは同じです。私はちょうど正確な例外とそれが起こった場所を送信することが理にかなっているかどうかを知りたがっています。他の提案のいくつかは一番上にキャッチすることですが、例外が発生した場所を失うように見え、スタックトレースでそれを掘り起こす必要があります。これは自動化するのが楽しいことではありません。私はこれまでのところ、この答えが一番好きです。 – brandon

0

各catchブロックの本体は同じです。タイプ固有のアクションを取っていないので、この場合は特定の例外タイプをキャッチする必要はありません。これは良いです:

try 
    { 
     this.data = CSVReader.ToDataTable(file); //throws a few basic exceptions 
    } 
    catch (Exception e) 
    { 
     throw new FailedLoadingCSVException(e, currentLine, file); 
    } 
+0

例外をマッピングする方法はありません。本当の例外が欲しいのであれば、e.messageレベルでキャプチャするだけではなく、同じ猫をスキンケアする多くの方法があります。 – MethodMan

+0

これを行うと、発生した場所が失われます。私にとって最も重要なのは、それが起こった場所で論理的にです。私はこれをユーザーに指示に翻訳する必要があります。だから、もし私が「引数がヌルになることはできない」と言えば、ファイル名のために、CSVなどでフィールドが見つからなかったということを意味するのかどうかわからないでしょう。スタックトレースで見つけることができますが、論理的に正しい場所にキャッチするのに役立つようです。私は知りませんが、私はそれを理解しようとしています。 – brandon

+0

@brandon:これはどこで発生しますか?それはあなたが持っているのと同じ論理です!あなたのロジックはむしろ 'if(xはint)を返すようなものです。x.ToString(); if(xが短い場合)return x.ToString();もし(xがdoubleなら)x.ToString(); 'それとも、currentLineと何か関係がありますか? – phoog

-2
try 
{ 
    this.data = CSVReader.ToDataTable(file); //throws a few basic exceptions 
} 
catch (Exception e) 
{ 
    Console.WriteLine(e.Message);   
} 
//unless you know what the exact Exception of the CSVReader is you can always just use Base Exception Class.. 
関連する問題