The .NET framework is littered with the following code (e.g. from the System.Guid class):
public static Guid ParseExact(String input, String format) { if (input == null) throw new ArgumentNullException("input"); if (format == null) throw new ArgumentNullException("format"); if (format.Length != 1) { // all acceptable format strings are of length 1 throw new FormatException(…); } … }
A common scenario when creating a publicly accessible method is to validate the parameters being passed into the method and throw an exception when something is amiss. I used this example as Guid.ParseExact was only added in .NET 4, showing that this is still the ‘Microsoft way’.
But when you do this all over the framework it impacts on performance. The reality is that in the vast majority of cases this code should be called with a valid string representation of a Guid, at least with a non-null string (with length > 0), yet we have to do the checks every time the method is called. Even if I can be certain the string represents a real Guid, there is no way for me to skip these checks. You might say that’s not really a problem, but if I’m writing an ORM (for example) and I’m looking for raw object creation throughput, that overhead starts to become significant.
When ReSharper looked at this problem, they realised that it would be possible to indicate whether null values were expected or not using attributes. For that reason they added a bunch of attributes that they use to inform their value analysis module, and then they went through the framework and effectively marked it all up. If you’re using ReSharper (and if not, why not?) you can make use of this code analysis by modifying the above code:
public static Guid ParseExact( [NotNull]String input, [NotNull]String format) { if (format.Length != 1) { // all acceptable format strings are of length 1 throw new FormatException(…); } … }
ReSharper will test any code that uses ParseExact and provide a helpful squiggle if it things you are passing in a null when you shouldn’t be. To be fair the intent here is not to remove the checks themselves, but to make their value analysis not bug you with warnings when they’re unnecessary, but the temptation to remove the checks is powerful. But this has two big problems.
- What is the person using your library doesn’t have ReSharper?
- What if the person using your library ignores the squiggle?
What ReSharper is doing is pretty smart, by analysing the code it can tell if it’s possible a null is sneaking through. This isn’t done by running the code, but rather by analysing paths through the code. This can be done at compile time – though it’s not always bomb proof. When Microsoft Research approached this problem they came up with an even better solution – Code Contracts.
With Code Contracts enabled it is possible to rewrite the above like so :
public static Guid ParseExact(String input, String format) { Contract.Requires(input != null); Contract.Requires(format != null); Contract.Requires(format.Length != 1); ... }
Which looks eerily like Debug.Assert or Assert.IsTrue statements. Unlike those two examples though the Contract namespace includes methods that effectively run at compile time to validate constraints. These methods can also optionally run at runtime, which is useful when a constraint cannot be fully validated at compile time. Using contracts allows us to move away from those chains of parameter validation code safely. It even lets us provide information about the outputs of the method using the Contract.Ensures method. This is a hugely powerful tool in the .NET developer’s arsenal.
But it still suffers from a similar problem to the ReSharper solution – what if the person using your library doesn’t use Code Contracts? Also, unlike ReSharper, it also modifies the assembly at build time which can be buggy when using other tools, such as PostSharp. The final nail in the coffin is that ReSharper just doesn’t support Code Contracts in any useful way, and so you’re left implementing both. It’s for this reason I have repeatedly come back to Code Contracts over the last few years and debated whether to include it in projects.
However, the community does seem to be swinging towards it’s adoption and .NET 4.5 includes the System.Diagnostics.Contract namespace out of the box – but you currently still need to install the Code Contracts extension into VS11 beta if you want to configure it (note if you’re using VS11 that you currently also have to execute ‘devenv /setup’ to complete installation which is a bit of a mess). If you’re targeting the new framework then there’s no reason to not include Code Contracts as you’re end users won’t have compilation problems – but you may want to encourage them to set up their projects to make use of the contracts you define. Maybe the Visual Studio team will include the extension when VS11 goes RTM, but that’s not clear.
Until they do, sometimes my code will include examples whilst other times you’ll see the ReSharper attributes. if you struggle compiling the code you can read the following article, or install the Nuget. If you want to go old school just create fake attributes that match the signatures used. If all else fails, you can safely delete the attributes.
The main problems the annotations is they add clutter to the code which hinders readability. Therefore, if you trust your team, there is little need to use them in in-house projects just as there is little need to do null checks all over the place. Good unit tests will protect against code monkeys making a hash off your code too.
The game changes when we talk about public APIs. Personally, I prefer the annotations to null checks as they appear once in the signatures and then the rest of the code has less noise.
Code contracts, with compile time checking are pretty useful.
I don’t believe the annotations hinder readability, but the contracts potentially do – that said they take less space and are arguably more readable than the null checks, as you allude to. Both options improve usability though – the annotations particularly when used with realtime value analysis help prevent coding bugs.
Code contracts have the advantage of appearing in a ‘contract block’ at the start of a method, and with modern VS persistent outlining this can be easily hidden – unlike the annotations. However the annotations can be declared in an external XML file (which hinders maintenance) and can also be declared on interfaces – whereas the contracts would require an abstract base class (for example) which undermines the performance benefits.
Both solutions are clearly superior to the if..then checks and it’s likely code contracts will win the long game now they are in the framework libraries.
Obviously the readability issues are just an opinion, maybe you get used to it. I don’t disagree with the rest of your points.
Another way of dealing with null checking is to use the Null Object design pattern. I don’t know if any of you have heard of it, but just in case you haven’t see the link below 🙂
http://www.codeguru.com/csharp/.net/net_general/patterns/article.php/c16503/Null-Object-Pattern-in-C.htm
Baz