Checking the argument of a complex type method
I'm a fan of validating arguments in methods. Something like this is commonly seen in my code:
public void Foo(string someString)
{
#region parameter validation
if(string.IsNullOrWhiteSpace(someString))
throw new ArgumentNullException("someString");
// Let ignore the .NET 4.6 nameof-feature for the sake of this question.
#endregion
// do things
}
Now let's say the method doesn't accept a string, but a complex object. Let's say I want a method to check if a property of this complex object is set. What would be the correct way to do this? So far I've done it like this:
public void Foo(Person person)
{
#region parameter validation
if(person == null)
throw new ArgumentNullException("person");
if(string.IsNullOrWhiteSpace(person.Name))
throw new ArgumentNullException("person.Name");
#endregion
// do things
}
Is there any best practice for this? Is this the ArgumentNullException
correct way? Are these properties checked as recommended?
source to share
There doesn't seem to be any right correct way to do this, so I'll stick with my own path, which is
if(myComplexObject.SomeProperty == null)
throw new ArgumentNullException("myComplexObject.SomeProperty");
The subscriber will immediately know what is wrong.
Note that I am not using the nameof-operator because this again will just give me SomeProperty
instead myComplexObject.SomeProperty
. I could do nameof(myComplexObject) + "." + nameof(myComplexObject.SomeProperty
, but I need to give some more considerations, at first glance it doesn't look right.
And here are my reasons why I think other approaches are worse than mine:
Some said that I should check in the model, which I won't do because the model doesn't know how the object should be checked for methods that it doesn't know. If this is the general reality of an instance of an object, be sure to insert it into the model or if you ask me even about the business layer to validate the business, but this is not the case.
Others said that I can just use the zero coalescing operator ( ?.
in C #), which I also won't do because it's not a "if null then just work with this" scenario, but "I won't do anything if it's null and you must feel bad for giving me a null, fix your code. "
Then there are code contracts that provide a sweet way to validate arguments and have some nicer advantages, but ultimately the same question arises: how can I tell that a caller whose value is a complex object is invalid? He needs to know that the X property of the Y argument is wrong, nothing else. While code contracts are not the answer to my question, I think my answer could also be used there if the same situation arises.
And then it was suggested that I shouldn't be checking the properties of the objects in that place, but in the function that needs it. The point is that a property is barely used to be passed as a separate distinct value to another function. Plus, it seemed to blow the idea of interfaces.
source to share
In my case, all my models have a Validate method, so I just call it, and if it returns false (which means I got errors), I call another method to get errors from it and display to the user, something like:
if (person == null)
throw new ArgumentNullException (nameof (person));
// Validates only 1 property.
if (!person.Validate (nameof (person.MyProperty)))
DisplayErrors (person);
// Validates all properties.
if (!person.Validate ())
DisplayErrors (person);
The reason for this is that in most cases I have to allow the models to be invalid while the user fills in this information. I am using this method with the INotifyDataErrorInfo interface for WPF to allow the UI to update when errors occur.
If a method or operation only needs a part of an object, the facade model is the solution where you create a new class that will convert your more complex one, and the facade class also has a Validate method for partial validation.
source to share
You can consider code contracts . Of course, validation, as you have shown, is a great way to validate parameters ... assuming you are only validating the fields you are using. Validating for fields you don't reference causes problems (unless that is the explicit purpose of the method).
The newest version of C # contains '?.' operator that allows you to do things like this:
int? length = customers?.Length;
or
string name = person?.Name;
This allows you to protect yourself from null reference exceptions, and in some cases may be preferable to prechecking.
Oh, and when running a code analysis tool, it is common to use flags using parameters before they are checked, so at least these tools consider this to be best practice.
Update (in response to comment): It depends a lot on what you are trying to do. If you are building an API and want to return a better error message, then what you assumed is probably about what you can do. This assumes that you are primarily interested in communicating with your API consumer.
If your primary interest is that the data provided to your method is valid and will not badly influence the behavior of your method (CYA), you should look into contracts. Contracts have the advantage of providing message conditions (ensuring that your method doesn't return something unexpected) and invariant conditions (ensuring that you don't change what you didn't intend). Contracts can provide both static (compile-time) assurances and run-time guarantees. There is a rather subtle way of managing contract options. Breaches of contract time over time cause contract exceptions that people may be less familiar with. Compilation complaints may also be unfamiliar to users. This implies,that you are mainly interested in making sure it works right or not being accused of misusing your API.
You can use both, for example using Contracts for compile-time checks and simpler if-checks for runtime.
source to share
This sounds like a design problem to me 99% of the time. Is the person valid without a name? I suspect not. And if not, why are you allowing one to be created without it? And if you (for some reason) allow it, is it not responsible for the object to keep its own "validity"? I would rely on something like data annotations to mark up the metadata of the required states and just check that the one passed in personally is "valid"
source to share