4

I'm using Code Contracts in C# 4.0. I'm applying the usual static method chaining to simulate optional parameters (I know C# 4.0 supports optional parameters but I really don't want to use them).

The thing is that my contract requirements are executed twice (or possibly the number of chained overloads I'd implement) if I call the Init(string , string[]) method -- an obvious effect from the sample source code below. This can be expensive, especially due to relatively expensive requirements like the File.Exists I use.

public static void Init(string configurationPath, string[] mappingAssemblies)
{
    // The static contract checker 'makes' me put these here as well as
    // in the overload below.
    Contract.Requires<ArgumentNullException>(configurationPath != null, "configurationPath");
    Contract.Requires<ArgumentException>(configurationPath.Length > 0, "configurationPath is an empty string.");
    Contract.Requires<FileNotFoundException>(File.Exists(configurationPath), configurationPath);
    Contract.Requires<ArgumentNullException>(mappingAssemblies != null, "mappingAssemblies");
    Contract.Requires<FileNotFoundException>(Contract.ForAll<string>(mappingAssemblies, (n) => File.Exists(n)));

    Init(configurationPath, mappingAssemblies, null);
}

public static void Init(string configurationPath, string[] mappingAssemblies, string optionalArgument)
{
    // This is the main implementation of Init and all calls to chained
    // overloads end up here.
    Contract.Requires<ArgumentNullException>(configurationPath != null, "configurationPath");
    Contract.Requires<ArgumentException>(configurationPath.Length > 0, "configurationPath is an empty string.");
    Contract.Requires<FileNotFoundException>(File.Exists(configurationPath), configurationPath);
    Contract.Requires<ArgumentNullException>(mappingAssemblies != null, "mappingAssemblies");
    Contract.Requires<FileNotFoundException>(Contract.ForAll<string>(mappingAssemblies, (n) => File.Exists(n)));

    //...
}

If however, I remove the requirements from that method, the static checker complains that the requirements of the Init(string, string[], string) overload are not met. I reckon that the static checker doesn't understand that there requirements of the Init(string, string[], string) overload implicitly apply to the Init(string, string[]) method as well; something that would be perfectly deductable from the code IMO.

This is the situation I would like to achieve:

public static void Init(string configurationPath, string[] mappingAssemblies)
{
    // I don't want to repeat the requirements here because they will always
    // be checked in the overload called here.
    Init(configurationPath, mappingAssemblies, null);
}

public static void Init(string configurationPath, string[] mappingAssemblies, string optionalArgument)
{
    // This is the main implementation of Init and all calls to chained
    // overloads end up here.
    Contract.Requires<ArgumentNullException>(configurationPath != null, "configurationPath");
    Contract.Requires<ArgumentException>(configurationPath.Length > 0, "configurationPath is an empty string.");
    Contract.Requires<FileNotFoundException>(File.Exists(configurationPath), configurationPath);
    Contract.Requires<ArgumentNullException>(mappingAssemblies != null, "mappingAssemblies");
    Contract.ForAll<string>(mappingAssemblies, (n) => File.Exists(n));

    //...
}

So, my question is this: is there a way to have the requirements of Init(string, string[], string) implicitly apply to Init(string, string[]) automatically?

Update

I was using the ForAll method in the wrong way: it is intended to use inside a requirement or alike, like this:

Contract.Requires<FileNotFoundException>(Contract.ForAll<string>(mappingAssemblies, (n) => File.Exists(n)));
Sandor Drieënhuizen
  • 6,310
  • 5
  • 37
  • 80
  • 1
    related to/duplicate of: http://stackoverflow.com/questions/2539497/code-contracts-do-we-have-to-specify-contract-requires-statements-redundant/2626997 If you want to remove overhead, in your release build you can generate a contract reference assembly and enable call-site checking - see the docs for more detail. – porges May 05 '10 at 22:59
  • On a side note, File.Exists is a poor choice for a contract condition, as the caller cannot statically guarantee this condition. This should be documented as an exception case, but it's not something that should be part of the binding contract, as it could never be guaranteed to be satisfied. – Dan Bryant Oct 21 '11 at 15:50
  • Thanks, you're totally right about that. A better assertion would be validating paths syntactically instead, using a regular expression or something. – Sandor Drieënhuizen Oct 22 '11 at 14:53

2 Answers2

1

I don't think there is for the general case.

In your specific case, you could certainly move the more expensive ForAll(File.Exists) to the actual implementation method and get no warnings :

        public static void Init(string configurationPath, string[] mappingAssemblies)
    {
        Contract.Requires<ArgumentNullException>(configurationPath != null, "configurationPath");
        Contract.Requires<ArgumentException>(configurationPath.Length > 0, "configurationPath is an empty string.");
        Contract.Requires<FileNotFoundException>(File.Exists(configurationPath), configurationPath);
        Contract.Requires<ArgumentNullException>(mappingAssemblies != null, "mappingAssemblies");
        Init(configurationPath, mappingAssemblies, null);
    }

    public static void Init(string configurationPath, string[] mappingAssemblies, string optionalArgument)
    {
        Contract.Requires<ArgumentNullException>(mappingAssemblies != null, "mappingAssemblies");
        Contract.ForAll<string>(mappingAssemblies, (n) => File.Exists(n));
    }

Edit - I would forget about doing it at the earlier levels and just decorate the methods with the ContractVerification() attribute. This gives me no warnings, 7 checked assertions, with all the static checking options turned on.

    [ContractVerification(false)]
    public static void Init(string configurationPath, string[] mappingAssemblies)
    {
        Init(configurationPath, mappingAssemblies, null);
    }

    public static void Init(string configurationPath, string[] mappingAssemblies, string optionalArgument)
    {

        Contract.Requires<ArgumentNullException>(mappingAssemblies != null, "mapping assemblies");
        Contract.Requires<ArgumentNullException>(configurationPath != null, "configurationpath");
        Contract.Requires<ArgumentException>(configurationPath.Length > 0, "configurationPath is an empty string");
        Contract.Requires<FileNotFoundException>(File.Exists(configurationPath));
        Contract.Requires<FileNotFoundException>(Contract.ForAll<string>(mappingAssemblies, (n) => File.Exists(n)));


        // ... 
    }
Tom
  • 1,269
  • 8
  • 12
  • Thanks for the suggestion, it indeed appears not to generate a warning. I assumed that it had to be in every chained method. I just discovered that a plain File.Exists has the effect of generating a warning while a ForAll(File.Exists) doesn't. – Sandor Drieënhuizen May 02 '10 at 17:41
  • I found out that was using the `ForAll` method in the wrong way (see Update in the question). Now that I've corrected it, it gives the same problem as the rest of the requirements. – Sandor Drieënhuizen May 02 '10 at 18:51
  • Answer updated. It might be best to forget about trying to require that at the earlier levels and just propagate it downwards and check there. – Tom May 02 '10 at 19:54
  • Thanks, I guess the `ContractVerification` attribute suits me well. – Sandor Drieënhuizen May 02 '10 at 20:35
  • The problem with this solution is that the static analyzer no longer thinks `Init(string,string[])` has any contracts and will not warn you if any code calls this method with null values for example. `ContractVerification` should really only be used when the analyzer cannot understand your contracts correctly or you are working with alot of uncontracted code. – Brandon Sep 21 '11 at 14:06
0

Since you are using Contracts I assume you are using C# 4.0. Then you can use optional parameters instead and only have one place to put your contracts.