Fluent programming gets a bad reputation, since some developers like to write code like the following:
var time = 7.Days().Plus(4.Hours())
Barf. However, when used properly, I think it’s very powerful. Let’s look at a typical method with some parameter validation:
// Copy src[srcOffset, srcOffset + length) into dst[dstOffset, dstOffset + length)
public static void Copy<T>(T[] dst, long dstOffset, T[] src, long srcOffset, long length)
{
if (dst == null)
throw new ArgumentNullException(“dst”);
if (src == null)
throw new ArgumentNullException(“src”);
if (dstOffset + length > dst.Length || dstOffset < 0)
throw new ArgumentOutOfRangeException(
“dst, dstOffset, length”,
string.Format(“dst range is not within bounds, ({0} + {1}) > {2}”, dstOffset, length, dst.Length));
if (srcOffset + length > src.Length || srcOffset < 0)
throw new ArgumentOutOfRangeException(
“src, srcOffset, length”,
string.Format(“src range is not within bounds, ({0} + {1}) > {2}”, srcOffset, length, src.Length));
if (length < 0)
throw new ArgumentOutOfRangeException(“length”, “length must be >= 0, ” + length.ToString());
for (int di = dstOffset; di < dstOffset + length; ++di)
dst[di] = src[di – dstOffset + srcOffset];
}
That’s a lot of code for parameter validation, but in a robust system it is necessary. For debugging purposes, having all the information in there with the actual parameter values is invaluable, so that you can get a stack trace that tells you, “Length was too big. It was 50, but the max was 49.”
The problem here is twofold. One, code like this gets sprinkled all over the codebase of a large application and so it gets repetitive, tiresome, and is a bug hazard. Having an off-by-1 error is many times worse if it’s in your validation code. Or, because it’s tiresome, sometimes there just won’t be any validation.
The second problem is actually much more subtle. Ask yourself this: if both src and dst are null, what exception does the caller get? (and subsequently, what goes into the crash log or Watson upload?) It will only tell you that dst is null. This leads to more iterations in debugging than is optimal, where you fix the problem of dst equaling null only to immediately get it crashing on you again when src is null. If the exception told you about both errors, you could have saved a lot of time.
This happens more often than I’d like when debugging issues on other people’s systems, especially ones I don’t have any direct access to (physical or remote, ala Remote Desktop). The end-user will post a Paint.NET crashlog to the forum, I’ll fix it and send them a patch or new build, and then the same method will crash on the very next line of code. This is especially relevant to methods for graphics which take parameters for stuff like width, height, location, bounding box, etc. The X value may be bad, but the Y value might also be bad. I need to know about both, along with the valid ranges (and not just “out of range”).
There are times where I have fixed issues with no direct interaction with a user: if I get a bunch of crash logs for a certain issue, but I can’t reproduce it, I have often been able to fix it by incorporating a hopeful and conservative fix into the next release and then monitoring to make sure that no more crash logs come in. And yes, I’ve done that many times with Paint.NET.
Reporting an aggregated judgement like this is just not fun. To go the extra mile you need to create a StringBuilder, decide on the preciding exception type, manage concatenation of multiple parameter names (“sentence-ization”), etc. Like this …
public static void Copy<T>(T[] dst, long dstOffset, T[] src, long srcOffset, long length)
{
StringBuilder sb = new StringBuilder();
if (dst == null)
sb.Append(“dst. “);
if (src == null)
sb.Append(“src. “);
if (sb.Length > 0)
throw new ArgumentNullException(sb.ToString());
if (dstOffset + length > dst.Length || dstOffset < 0)
…
if (srcOffset + length > src.Length || srcOffset < 0)
…
if (length < 0)
…
if (sb.Length > 0)
throw new ArgumentOutOfRangeException(sb.ToString());
…
}
Boo. This is still tiresome, and creates extra objects, etc. Because of the extra work involved, this tends to be done reactively instead of proactively. Only the “hot” methods get the comprehensive logic.
I’ve come up with another method. Check this out:
public static void Copy<T>(T[] dst, long dstOffset, T[] src, long srcOffset, long length)
{
Validate.Begin()
.IsNotNull(dst, “dst”)
.IsNotNull(src, “src”)
.Check()
.IsPositive(length)
.IsIndexInRange(dst, dstOffset, “dstOffset”)
.IsIndexInRange(dst, dstOffset + length, “dstOffset + length”)
.IsIndexInRange(src, srcOffset, “srcOffset”)
.IsIndexInRange(src, srcOffset + length, “srcOffset + length”)
.Check();
for (int di = dstOffset; di < dstOffset + length; ++di)
dst[di] = src[di – dstOffset + srcOffset];
}
Yow! Ok that’s much easier to read. And here’s the kicker: if no problems are found with your parameters, then no extra objects are allocated. The cost for this pattern is only in the extra method calls.
There are three classes involved here: Validate, Validation, and ValidationExtensions. Here’s the Validate class:
public static class Validate
{
public static Validation Begin()
{
return null;
}
}
That was easy. This allows us to not allocate a “Validation” object, and its enclosed fields, until we actually encounter a problem. The presiding philosophy in code that uses exception handling is to optimize for the non-exceptional code path, and that’s exactly what we’re doing here. Here’s the Validation class:
public sealed class Validation
{
private List<Exception> exceptions;
public IEnumerable<Exception> Exceptions
{
get
{
return this.exceptions;
}
}
public Validation AddException(Exception ex)
{
lock (this.exceptions)
{
this.exceptions.Add(ex);
}
return this;
}
public Validation()
{
this.exceptions = new List<Exception>(1); // optimize for only having 1 exception
}
}
It’s basically just a list of exceptions. AddException() returns ‘this’ to make some of the code in the ValidationExtensions class easier to write. Check it out:
public static class ValidationExtensions
{
public static Validation IsNotNull<T>(this Validation validation, T theObject, string paramName)
where T : class
{
if (theObject == null)
return (validation ?? new Validation()).AddException(new ArgumentNullException(paramName));
else
return validation;
}
public static Validation IsPositive(this Validation validation, long value, string paramName)
{
if (value < 0)
return (validation ?? new Validation()).AddException(new ArgumentOutOfRangeException(paramName, “must be positive, but was ” + value.ToString()));
else
return validation;
}
…
public static Validation Check(this Validation validation)
{
if (validation == null)
return validation;
else
{
if (validation.Exceptions.Take(2).Count() == 1)
throw new ValidationException(message, validation.Exceptions.First()); // ValidationException is just a standard Exception-derived class with the usual four constructors
else
throw new ValidationException(message, new MultiException(validation.Exceptions)); // implementation shown below
}
}
}
The sum of these collections allows us to write validation code in a very clean and readable format. It reduces friction for having proper validation in more (or all? :)) methods, and reduces the bug hazard of either incorrect or omitted validation code.
Missing from this implementation, and other kinks to work out:
- Could use lots of additional methods within ValidationExtensions. (some were omitted for brevity in this blog post)
- Calling ValidationExtensions.Check() is itself not validated. So, if you forget to put a call to it at the end of your validation expression then the exception will not be thrown. Often you’ll end up plowing into a null reference and getting a NullReferenceException, especially if you were relying on ValidationExtensions.IsNotNull(), but this isn’t guaranteed for the other validations (esp. when dealing with unmanaged data types). It would be simple to add code to Validation to ensure that its list of exceptions was “observed”, and if not then in the finalizer it could yell and scream with an exception.
- The exception type coming out of any method that uses this will be ValidationException. This isn’t an issue for crash logs, but it is for when you call a method and want to discriminate among multiple exception types and decide what to do next (e.g., FileNotFoundException vs. AccessDeniedException). I’m sure there’s a way to fix that, with better aggregation, and (hopefully) without reflection.
- Should probably change the IEnumerable<Exception> in Validation to be Exception[].
Here’s the implementation of MultiException, as promised in the code above. And, in fact, it’s incomplete because it does not print all of the exceptions in a ToString() type of call. Umm … how about I leave that as an exercise for the reader? 🙂
[Serializable]
public sealed class MultiException
: Exception
{
private Exception[] innerExceptions;
public IEnumerable<Exception> InnerExceptions
{
get
{
if (this.innerExceptions != null)
{
for (int i = 0; i < this.innerExceptions.Length; ++i)
{
yield return this.innerExceptions[i];
}
}
}
}
public MultiException()
: base()
{
}
public MultiException(string message)
: base()
{
}
public MultiException(string message, Exception innerException)
: base(message, innerException)
{
this.innerExceptions = new Exception[1] { innerException };
}
public MultiException(IEnumerable<Exception> innerExceptions)
: this(null, innerExceptions)
{
}
public MultiException(Exception[] innerExceptions)
: this(null, (IEnumerable<Exception>)innerExceptions)
{
}
public MultiException(string message, Exception[] innerExceptions)
: this(message, (IEnumerable<Exception>)innerExceptions)
{
}
public MultiException(string message, IEnumerable<Exception> innerExceptions)
: base(message, innerExceptions.FirstOrDefault())
{
if (innerExceptions.AnyNull())
{
throw new ArgumentNullException();
}
this.innerExceptions = innerExceptions.ToArray();
}
private MultiException(SerializationInfo info, StreamingContext context)
: base(info, context)
{
}
}
Paint.NET Blog 