Code contracts and overloads

Question

Let's say I'm writing an utility library that defines a method with two overloads :

public static class MyClass
{
    public static void DoSomething(string myValue, bool myFlag)
    {
        Contract.Requires<ArgumentNullException>(myValue != null);
        CallExternalMethod(myValue);
        if(myFlag){
            AlsoCallOtherMethod(myValue);
        }        
    }

    public static void DoSomething(string myValue)
    {
        Contract.Requires<ArgumentNullException>(myValue != null);
        DoSomething(myValue, true);
    }
}

As a best practice, I define the logic in only one method, and use overloads to specify default parameters (.Net 3.5, I can't use the .Net 4 default value parameters).

As you can see, I'm also validating the input by using contracts.

As the logic is only in the first method, is the second Contract useless?

Concerning the runtime check, I know it is useless, but what about the static checker? Is it smart enough to understand my pattern?

Answer 1

I completely disagree with BonyT's answer.

Code Contracts are part of your documentation and the method's contract, just like types and names. There is no reason for a user/developer to assume that you just call the other method overload and nothing else. It should not be a guessing game to find out which contracts apply to a method. If there is a constraint on an argument or return value, document it using Code Contracts, even if you internally - hidden to everyone else - just call another method.

You must not think of Code Contracts as just an assertion tool: the control flow runs to the other overload, which checks the argument, so I don't have to do it here. You must see it as documentation.

And on a practical level: adding Code Contracts also adds those contracts to the XML documentation you generate. And tools such as the static checker and the Code Contracts Editor Extensions can show the contract to users. Some other good arguments can be found in this related post by Pascal Cuoq and this post by Jon Skeet.

However, it is redundant from a runtime perspective. As to the runtime-overhead: most of the checks will be relatively small and of no performance concern. Some checks may be of performance concern (most notably checks on collections, arrays and enumerables) but these can be disabled separately in the Code Contracts properties dialog of your project (for example for the Release build). The same holds for contracts in general: you can lower the checking to any level you want, as to improve performance when that is really critical. Users will still get static checking if you build a contract assembly.

To summarize. Con's of specifying redundant Requires and Ensures:

• Same condition might be checked twice or more
• Tiny negative performance impact due to the multiple checking (if you don't disable it)
• More typing (but see these snippets)

Pro's:

• Code documentation
• Documentation in generated XML
• Static checker for users of your code works as expected
• Code Contracts Editor Extensions works
• ArgumentException for Requires occur in the method that was called instead of some method deep down far away