I had a discussion with a colleague regarding the usage of Code Contracts to perform prerequisites checks.
Let's say we have the following code:
namespace Project
{
using System;
using System.Diagnostics.Contracts;
public class Thing
{
public string Foo { get; set; }
public int Bar { get; set; }
}
public class ThingsManipulator
{
public void AddThing(Thing thing)
{
Contract.Requires<ArgumentNullException>(thing != null);
// Do something
}
}
}
If in the // Do something
I'm accessing thing.Foo
and thing.Bar
to do things, should I validate them too through Code Contracts?
public void AddThing(Thing thing)
{
Contract.Requires<ArgumentNullException>(thing != null);
Contract.Requires<ArgumentException>(!string.IsNullOrWhiteSpace(thing.Foo));
Contract.Requires<ArgumentException>(thing.Bar > 0);
// Do something
}
My colleague says that only the parameter as a whole should be checked (i.e. we should only place the first contract), I think methods should check what they use regardless if it's the whole parameter or one of its properties (i.e. we should all the three contracts).
Please note, I do understand and agree with that if the properties of a parameter should always fulfill a requirement, that requirement should be placed in the object's invariant checks.
What I'm referring to are values which are generally valid, but not valid for a particular method (e.g. in the example above thing.Bar
may happily hold negative values, but AddThing
does not like them).
My colleague says in these cases the method signature should explicit all the items it's using instead of a single object (e.g. AddThing(string thingFoo, int thingBar)
), and run the checks on them.
So:
- Should we validate what a method uses or only the parameters as a whole and "explode" the parameters?
- Is there a technical reason to do it (regardless of whatever "it" means) or is it a matter of preference?
I haven't been able to find guidelines about this in the manual, maybe I missed something?