tags:

views:

61

answers:

2

Far as best practices are concerned, which is better:

public void SomeMethod(string str) 
{
    if(string.IsNullOrEmpty(str))
    {
        throw new ArgumentException("str cannot be null or empty.");
    }

    // do other stuff
}

or

public void SomeMethod(string str) 
{
    if(str == null) 
    {
        throw new ArgumentNullException("str");
    }

    if(str == string.Empty)
    {
        throw new ArgumentException("str cannot be empty.");
    }

    // do other stuff
}

The second version seems more precise, but also more cumbersome than the first. I usually go with #1, but figured I'd check if there's an argument to be made for #2.

+2  A: 

I would suggest using the first one. If your method doesn't expects null or empty string it really doesn't matter if null or empty was passed - important to report and error and that is what 1st variant does.

Andrew Bezzub
"If your method doesn't expects null or empty string it really doesn't matter if null or empty was passed" +1 as this is pretty much exactly what I was thinking.
kekekela
+2  A: 

I'd say the second way is indeed more precise - yes, it's more cumbersome but you can always wrap it in a method to avoid having to do it all the time. It could even be an extension method:

str.ThrowIfNullOrEmpty("str");


public static void ThrowIfNullOrEmpty(this string value, string name)
{
    if (value == null)
    {
        throw new ArgumentNullException(name);
    }
    if (value == "")
    {
        throw new ArgumentException("Argument must not be the empty string.",
                                    name);
    }
}

Another form which is potentially useful is one which returns the original string if everything is okay. You could write something like this:

public Person(string name)
{
    this.name = name.CheckNotEmpty();
}

Another option to consider is using Code Contracts as an alternative to throwing your own exceptions.

Jon Skeet