19

メソッドの入力を検証するときに、引数が null かどうかをチェックしていました。そうであれば、ArgumentNullException をスローします。リスト内のすべての引数に対してこれを行うため、最終的には次のようなコードになります。

 public User CreateUser(string userName, string password, 
                            string Email, string emailAlerts, 
                            string channelDescription)
    {

        if (string.IsNullOrEmpty(userName))
            throw new ArgumentNullException("Username can't be null");

        if (string.IsNullOrEmpty(Email))
            throw new ArgumentNullException("Email can't be null");
       //etc, etc, etc
    }

これでよろしいですか?なぜ私はこれをしなければならないのですか?例外をスローする代わりに、単純にすべてのチェックをグループ化して null 値を返せばよいでしょうか? この状況に対処するためのベストプラクティスは何ですか?

PS:これを変更したいのは、メソッドが長いと非常に面倒になるからです。
アイデア?

4

8 に答える 8

17

私が使用し、NHibernate のソースから取り上げた可能性のあるアプローチはGuard、次のように使用される静的クラスを作成することです。

public void Foo(object arg1, string arg2, int arg3)
{
    Guard.ArgumentNotNull(arg1, "arg1");
    Guard.ArgumentNotNullOrEmpty(arg2, "arg2");
    Guard.ArgumentGreaterThan(arg3, "arg3", 0);
    //etc.
}

public static class Guard
{
    public static void ArgumentNotNull(object argument, string parameterName)
    {
        if (parameterName == null)
            throw new ArgumentNullException("parameterName");

        if (argument == null)
            throw new ArgumentNullException(parameterName);
    }
    //etc.
}

これにより、メソッドの開始時に多くのチャフが削減され、パフォーマンスが向上します。

于 2009-07-21T11:27:50.720 に答える
17

このようなもので ArgChecker クラスを作成します

  ArgChecker.ThrowOnStringNullOrEmpty(userName, "Username");

ここで、ThrowOnStringNullOrEmpty は

  public static void ThrowOnStringNullOrEmpty(string arg, string name)
  {
      if (string.IsNullOrEmpty(arg))
        throw new ArgumentNullException(name + " can't be null");
  }

次のように、params arg を使用して引数のリストを処理することもできます。

  public static void ThrowOnAnyStringNullOrEmpty(params string[] argAndNames)
  {
       for (int i = 0; i < argAndName.Length; i+=2) {
          ThrowOnStringNullOrEmpty(argAndNames[i], argAndNames[i+1]);
       }
  }

そして、このように呼び出します

  ArgChecker.ThrowOnAnyStringNullOrEmpty(userName, "Username", Email, "email");
于 2009-07-21T11:26:43.733 に答える
9

メソッド、それが何をする必要があるか、どのデータを使用するかについて考える必要があります。null 値が実際の障害状態を表している場合は、例外を使用してください。null 値が受け入れられる場合は、受け入れます。

design by contractの原則、具体的には機能の前提条件について考え、それらを強制する方法を標準化します (Matt と Lou の両方が回答で提案しているため、詳細に入る必要はありません)。

考慮すべきもう 1 つの重要な点は、メソッド シグネチャのサイズです。メソッドに多くのパラメーターがある場合、これはおそらく抽象化が不適切であることを意味します。パラメーターをコレクション オブジェクトにグループ化し、それらのオブジェクトをパラメーターとして使用すると、実行する必要があるパラメーター チェックの数を減らすことができます。それらを使用するすべての関数でそれらをチェックする代わりに、パラメーター チェックをそれらのオブジェクトに移動できます。

したがって、すべての関数に 10 個の関連パラメーターを渡す代わりに、すべての関数で使用されるいくつかのパラメーターを見つけ出し、それらをオブジェクトにパッケージ化し、そのオブジェクト メソッドに含めてパラメーターを検証します。これには、1 つのパラメーターに関するルールを更新する必要がある場合に、簡単に変更できるという利点があります。

于 2009-07-21T11:32:00.727 に答える
5

また、C# 3.0 開発者にとって、この null チェックをカプセル化する優れた方法は、拡張メソッド内にあります。

public void Foo(string arg1, int? arg2)
{
  arg1.ThrowOnNull();
  arg2.ThrowOnNull();
}

public static class extensions
{
    public static void ThrowOnNull<T>(this T argument) where T : class
    {
        if(argument == null) throw new ArgumentNullException();
    } 
}

そして、必要に応じて、いつでもそれをオーバーロードして、引数名を取ることができます。

于 2009-07-21T12:52:33.823 に答える
3

ルーの答えに対する小さな改善は、代わりにハッシュテーブルを使用することです。これは、文字列だけでなくオブジェクトもチェックすることを意味します。また、メソッドに入力して処理する方が良いです:

public static class ParameterChecker
{
    public static void CheckForNull(Hashtable parameters)
    {
        foreach (DictionaryEntry param in parameters)
        {
            if (param.Value == null || string.IsNullOrEmpty(param.Value as string))
            {
                throw new ArgumentNullException(param.Key.ToString());
            }
        }
    }
}

次のように使用します。

public User CreateUser(string userName, string password, string Email, string emailAlerts, string channelDescription)    
{
    var parameters = new Hashtable();
    parameters.Add("Username", userName);
    parameters.Add("Password", password);
    parameters.Add("EmailAlerts", emailAlerts);
    parameters.Add("ChannelDescription", channelDescription);
    ParameterChecker.CheckForNull(parameters);

    // etc etc
}
于 2009-07-21T11:59:34.603 に答える
2

パラメータ名を渡すだけで、元のアプローチに固執します。その理由は、これらのヘルパー プロシージャを書き始めると、誰もがヘルパー プロシージャの書き方にさまざまな規則を使い始めると問題になるからです。誰かがあなたのコードを調べたとき、コードをデバッグするときに、ヘルパー プロシージャが正しく記述されていることを確認する必要があります。

指はGrasshopperを入力するのに疲れますが、各引数を個別にチェックしてください:)フォロワーが予期しない ArgumentException を取得し、どの引数が失敗したかを判断するためだけにデバッグ実行から保存されると、フォロワーはあなたを祝福します.

于 2009-07-21T12:31:17.123 に答える
0

あなたへの私の最初のアドバイスはReSharperを入手することです。null値の可能性の問題がある場合、およびそれらをチェックする必要がない場合を通知し、マウスをクリックするだけでチェックが追加されます。そうは言っても...

intまたはboolをチェックする必要はありません。これらはnullにすることはできません。

文字列はstring.IsNullOrEmpty()..で確認できます。

それでもすべてのパラメーターをチェックすることにした場合は、コマンドデザインパターンとリフレクションを使用できますが、コードは不必要に不格好になるか、次を使用してメソッドごとに呼び出します。private myType myMethod(string param1、int param2、byte [] param3){CheckParameters( "myMethod"、{param1、param2、param3}); //残りのコード...

そして、ユーティリティクラスにこれを入れてください:

///<summary>Validates method parameters</summary>
///... rest of documentation
public void CheckParameters(string methodName, List<Object> parameterValues) 
{
    if ( string.IsNullOrEmpty(methodName) )
       throw new ArgumentException("Fire the programmer! Missing method name", "methodName"));

    Type t = typeof(MyClass);
    MethodInfo method = t.GetMethod(methodName);
    if ( method == null )
       throw new ArgumentException("Fire the programmer! Wrong method name", "methodName"));
    List<ParameterInfo> params = method.GetParameters();
    if ( params == null || params.Count != parameterValues.Count )
       throw new ArgumentException("Fire the programmer! Wrong list of parameters. Should have " + params.Count + " parameters", "parameterValues"));

    for (int i = 0; i < params.Count; i++ )
    {
            ParamInfo param = params[i];
            if ( param.Type != typeof(parameterValues[i]) )
                throw new ArgumentException("Fire the programmer! Wrong order of parameters. Error in param " + param.Name, "parameterValues"));
            if ( parameterValues[i] == null )
                throw new ArgumentException(param.Name + " cannot be null");
    }
} // enjoy
于 2009-07-21T12:02:08.387 に答える