views:

249

answers:

3

I create class libraries, some which are used by others around the world, and now that I'm starting to use Visual Studio 2010 I'm wondering how good idea it is for me to switch to using code contracts, instead of regular old-style if-statements.

ie. instead of this:

if (fileName == null)
    throw new ArgumentNullException("fileName");

use this:

Contract.Requires(fileName != null);

The reason I'm asking is that I know that the static checker is not available to me, so I'm a bit nervous about some assumptions that I make, that the compiler cannot verify. This might lead to the class library not compiling for someone that downloads it, when they have the static checker. This, coupled with the fact that I cannot even reproduce the problem, would make it tiresome to fix, and I would gather that it doesn't speak volumes to the quality of my class library if it seemingly doesn't even compile out of the box.

So I have a few questions:

  • Is the static checker on by default if you have access to it? Or is there a setting I need to switch on in the class library (and since I don't have the static checker, I won't)
  • Are my fears unwarranted? Is the above scenario a real problem?

Any advice would be welcome.


Edit: Let me clarify what I mean.

Let's say I have the following method in a class:

public void LogToFile(string fileName, string message)
{
    Contracts.Requires(fileName != null);
    // log to the file here
}

and then I have this code:

public void Log(string message)
{
    var targetProvider = IoC.Resolve<IFileLogTargetProvider>();
    var fileName = targetProvider.GetTargetFileName();
    LogToFile(fileName, message);
}

Now, here, IoC kicks in, resolves some "random" class, that provides me with a filename. Let's say that for this library, there is no possible way that I can get back a class that won't give me a non-null filename, however, due to the nature of the IoC call, the static analysis is unable to verify this, and thus might assume that a possible value could be null.

Hence, the static analysis might conclude that there is a risk of the LogToFile method being called with a null argument, and thus fail to build.

I understand that I can add assumptions to the code, saying that the compiler should take it as given that the fileName I get back from that method will never be null, but if I don't have the static analyzer (VS2010 Professional), the above code would compile for me, and thus I might leave this as a sleeping bug for someone with Ultimate to find. In other words, there would be no compile-time warning that there might be a problem here, so I might release the library as-is.

So is this a real scenario and problem?

+1  A: 

First, the static checker is really (as I understand it) only available in the ultimate/academic editions - so unless everyone in your organization uses it they may not be warned if they are potentially violating an invariant.

Second, while the static analysis is impressive it cannot always find all paths that may lead to violation of the invariant. However, the good news here is that the Requires contract is retained at runtime - it is processed in an IL-transformation step - so the check exists at both compile time and runtime. In this way it is equivalent (but superior) to a regular if() check.

You can read more about the runtime rewriting that code contract compilation performs here, you can also read the detailed manual here.

EDIT: Based on what I can glean from the manual, I suspect the situation you describe is indeed possible. However, I thought that these would be warninings rather than compilation errors - and you can suppress them using System.Diagnostics.CodeAnalysis.SuppressMessage(). Consumers of your code who have the static verifier can also mark specific cases to be ignored - but that could certainly be inconvenient if there are a lot of them. I will try to find some time later today to put together a definitive test of your scenario (I don't have access to the static verifier at the moment).

There's an excellent blog here that is almost exclusively dedicated to code contracts which (if you haven't yet seen) may have some content that interests you.

LBushkin
Yes, I know all that. What I'm wondering is what happens if the analysis is unable to verify that a parameter will always be non-null in my code, even though it will always be, and thus the code fails to compile. Let's say I put the class library up on my website, and you downloaded it, and having Ultimate, the static checker would kick in, and not compile my library, even though it runs just fine, ie. the runtime code would never throw a fit because the parameter is always fine. It's just the analysis that cannot verify it. Will it then fail to compile, for you?
Lasse V. Karlsen
Seems I have to go download a trial of ultimate and find out for sure.
Lasse V. Karlsen
+2  A: 

When both your LogToFile and Log methods are part of your library, it is possible that your Log method will not compile, once you turn on the static checker. This of course will also happen when you supply code to others that compile your code using the static checker. However, as far as I know, your client's static checker will not validate the internals of the assembly you ship. It will statically check their own code against the public API of your assembly. So as long as you just ship the DLL, you'd be okay.

Of course there is a change of shipping a library that has a very annoying API for users that actually have the static checker enabled, so I think it is advisable to only ship your library with the contract definitions, if you tested the usability of the API both with and without the static checker.

Please be warned about changing the existing if (cond) throw ex calls to Contracts.Requires(cond) calls for public API calls that you have already shipped in a previous release. Note that the Requires method throws a different exception (a RequiresViolationException if I recall correctly) than what you'd normally throw (a ArgumentException). In that situation, use the Contract.Requires overload. This way your API interface stays unchanged.

Steven
This would be for new class libraries, I would not change existing libraries like this. I also publish the complete source to my libraries, there's no revenue stream involved, which would also make it a very low threshold of pain for the user to just ditch it 2 minutes after downloading it, it's not like they paid anything for it so if it doesn't compile, find something else. It smells more and more like I should not use code contracts at all, except for projects with binary releases, and since I use all my class libraries myself as well, it just plain looks like I'll have to forego it.
Lasse V. Karlsen
In that case, take a look at my CuttingEdge.Conditions library (http://conditions.codeplex.com). Perhaps it is a nice fluent alternative to Code Contracts.
Steven
I've put it on my TOREAD-list, but the current class library I'm working on, a public standalone version of my IoC container, will not have any dependencies on anything but the .NET runtime. Thanks for the link though.
Lasse V. Karlsen
"it is possible that your Log method will not compile, once you turn on the static checker" is false. The static checker will *never* prevent compilation from completing. It will merely warn you about unproven pre-/post-requisites.
Porges
@Lasse: CuttingEdge.Conditions is especially useful for line of business applications, were dependencies with other libraries (such as Conditions) do not matter. I totally understand you don't want any external dependencies.
Steven
@Porges: You are right that it is no C# compiler error. However, in my opinion the static checker doesn't give _warnings_, it gives _errors_. But it indeed doesn’t prevent you from compiling and shipping the dll.
Steven
A: 

No; the static analyzer will never prevent compilation from succeeding (unless it crashes!).

The static analyzer will warn you about unproven pre-/post-conditions, but doesn't stop compilation.

Porges