No more than my online scratch pad.
A Better Guard Class
Recently I was working with some of the developers in a different office, I don’t normally see their code, so it was interesting to see how they are meeting various problems. Generally speaking, they are doing some nice looking work. The architecture that they are moving towards looks like it’s going a long way to solving a lot of problems, all good stuff.
One bit of code that I kept seeing really bothered me. It was a guard class that I kept seeing. I think it was a clone of a class from the here. The reason it bothered me was simply that it didn’t read well, I kept thinking that it was guarding against an exception. In reality it was guarding against an error condition and would throw an exception if the condition happened. For example:
Guard.Against<InvalidOperationException>(string.IsNullOrEmpty(serverName), "Server name must be provided")I would much rather something that reads like this:
Throw<InvalidOperationException> .WithMessage("Server name must be provided") .When(string.IsNullOrEmpty(serverName));Here’s my implementation of the class, thoughts?
public static class Throw<T> where T : Exception{ public static Thrower WithMessage(string message) { return new Thrower(message); }
public class Thrower { private readonly string message; public Thrower(string message) { this.message = message; }
public void When(Func<bool> predicate) { if (predicate()) { throw (T)Activator.CreateInstance(typeof(T), message); } }
public void When(bool condition) { if (condition) { throw (T)Activator.CreateInstance(typeof(T), message); } } }}
This all strikes me as a little overkill. I can’t see the advantage of this over a simple if and throw:
if string.IsNullOrEmpty(serverName)
throw new InvalidOperationException(“Server name must be provided.”);
If you’re going to add extra functionality, like logging etc, then it makes more sense, but even then it just feels like a lot of extra wording to remember for something really simple. Maybe that’s just me being out of touch with VS’s autocomplete, but it feels like something that could be a simple method call is becoming a bloated chain of events. Also, from an extendibility POV, what happens if you want to pass other data to the exception? Suddenly, you’re going to need up daisy chain the static methods etc.
Beer to discuss further? Pie tonight?
It’s about intent. Your example is functional, it will work and on many levels there is nothing wrong with it. However, it does not clearly express what you intend the code to do. Since the code will be read many more times than it’s coded I would rather it is slightly easier to read, in any places that I can.
Your point about adding additional data to the exception is a good one. I don’t think it would be very difficult to create something that reads like Throw.WithMessage(“fail”).AndTheData(someData).When(youSuck == true);