Какова лучшая практика, если один аргумент является нулевым? - PullRequest
17 голосов
/ 21 июля 2009

при проверке входных данных методов я использовал для проверки, является ли аргумент нулевым, и если да, я выкидываю 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
    }

Это нормально? Зачем мне это делать? Было бы хорошо, если бы я просто сгруппировал все проверки и вернул нулевое значение вместо того, чтобы выдавать исключение? Какова наилучшая практика для решения этой ситуации?

PS: Я хочу изменить это, потому что с длинными методами это становится очень утомительным.
Идеи?

Ответы [ 7 ]

17 голосов
/ 21 июля 2009

Подход, который я использую и который я мог использовать из источника 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.
}

Это сокращает много мусора в начале методов и работает хорошо.

17 голосов
/ 21 июля 2009

Создайте класс 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, например:

  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");
9 голосов
/ 21 июля 2009

Вы должны подумать о методе, что он должен делать и с какими данными. Если нулевые значения представляют фактические условия отказа, используйте исключения. Если допустимы нулевые значения, примите их.

Подумайте о принципах дизайн по контракту , в частности, каковы предварительные условия для вашей функции, и стандартизируйте способ их применения (что Мэтт и Лу оба предлагают в своих ответах, поэтому мне не нужно вдаваться в подробности).

Еще одна важная вещь, которую следует учитывать, - это размер сигнатур вашего метода. Если у вас много параметров для ваших методов, это, вероятно, означает, что у вас плохие абстракции. Вы можете сократить количество проверок параметров, которые необходимо выполнить, если вы группируете параметры вместе в объектах коллекции и используете эти объекты в качестве параметров. Вы можете перенести проверку параметров на эти объекты вместо того, чтобы проверять их в каждой функции, которая их использует.

Таким образом, вместо передачи десяти связанных параметров каждой функции, выясните, какие из них используются в каждой функции, упакуйте их в объект и включите в этот объект методы для проверки параметров. Это дает дополнительное преимущество, заключающееся в том, что его легко изменять в случае необходимости обновления правил, касающихся одного параметра.

5 голосов
/ 21 июля 2009

И для разработчиков C # 3.0 среди нас отличный способ инкапсулировать эту нулевую проверку внутри метода расширения.

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();
    } 
}

И если вы хотите, вы всегда можете перегрузить его, чтобы получить имя аргумента.

3 голосов
/ 21 июля 2009

Небольшим улучшением ответа Лу было бы использование вместо этого хеш-таблицы, это означает, что она проверяет объекты так же, как и просто строки. Также просто приятнее заполнять и обрабатывать в методе:

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
}
2 голосов
/ 21 июля 2009

Я бы придерживался вашего оригинального подхода, за исключением просто передачи имени параметра. Причина в том, что как только вы начинаете писать эти вспомогательные процедуры, возникает проблема, когда все начинают использовать разные соглашения о том, как они пишут вспомогательные процедуры. Когда кто-то просматривает ваш код, он должен проверить, правильно ли вы написали вспомогательную процедуру при отладке кода.

Продолжайте проверять каждый аргумент отдельно, хотя ваши пальцы устают от ввода Grasshopper :) Ваши последователи будут благословлять вас, когда получат неожиданное исключение ArgumentException, и будут сохранены из отладочного прогона, просто чтобы определить, какой аргумент потерпел неудачу.

0 голосов
/ 21 июля 2009

Мой первый совет тебе - найди ReSharper. Он сообщит вам, когда возникает проблема возможных нулевых значений, и когда нет необходимости проверять их, и одним щелчком мыши добавит проверку. Сказав это ...

Обычно вам не нужно проверять нулевые строки и целые числа, если только вы не получаете информацию из какого-либо устаревшего источника.

Строки могут быть проверены с помощью string.IsNullOrEmpty () ...

Если вы все еще решите, что хотите проверить каждый параметр, вы можете либо использовать шаблон проектирования Command и отражение, но ваш код будет излишне неуклюжим, либо использовать следующее и вызывать его для каждого метода: приватный myType myMethod (строка 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
...