There is a simple pattern that is extremely effective and increases source code readability greatly. Sadly this pattern is underused and many developers do not even think about it. I am talking of course the ThrowIf methods used to throw exceptions when validating state and arguments.
The pattern is simple – write specific methods to check conditions and throw exceptions and name them with ThrowIfCondition. For example if we are writing List<T> and want to implement the RemoveAt(int index) method we might validate the argument like this.
public void RemoveAt(int index)
{
if(index < 0 || index >= Count)
{
throw new ArgumentOutOfRangeException(nameof(index));
}
//actual logic…
}
We can extract that in a method that makes it slightly easier to read because
code that resembles English sentences is easier to readpublic void RemoveAt(int index)
{
ThrowIfIndexInvalid(index);
//actual logic
}
private void ThrowIfIndexInvalid(int index)
{
if(index < 0 || index >= Count)
{
throw new ArgumentOutOfRangeException(nameof(index));
}
}
This is a small improvement but it piles up when the same validation logic is used in multiple places. For example in the List<T> case the validation logic can be reused in places where an index is passed as an argument like Insert and the getter and setter for the indexer.
One place where the pattern shines particularly brightly is ASP.NET Web API. Web API has the concept of
HttpResponseException. When thrown this exception is caught by Web API and converted in the corresponding HTTP response. One can argue that this practice is using exceptions for flow control but a good counter-argument is that non-success status codes in HTTP are effectively exceptions so it is OK to represent them as exceptions in our backend code. In any case I have found that they work very well and the code becomes very readable when we combine these exceptions with the ThrowIf pattern. For example when you accept some data you check if it is valid like this
public IHttpActionResult Post(Comment comment)
{
if(!ModelState.IsValid)
{
return BadRequest();
}
//save the comment in the database or something
}
This is the normal flow that returns a result object and BadRequest is a helper method inherited from ApiController that returns a bad request result. But what if you want to return a normal object for example the comment with the populated ID. Then we can change the method like this:
public Comment Post(Comment comment)
{
if(!ModelState.IsValid)
{
throw new HttpResponseException(new HttpResponseMessage(HttpStatusCode.BadRequest));
}
//save the comment in the database and return it
}
And then we might want to put the validation errors in the response and this becomes a lot of code to repeat in half of your actions. So here are a couple of methods that I put in my base controller and use in my actions
protected void ThrowBadRequestIfModelStateInvalid()
{
if (!ModelState.IsValid)
{
var messageBuilder = new StringBuilder();
foreach (var modelItem in ModelState)
{
foreach (ModelError error in modelItem.Value.Errors)
{
messageBuilder.AppendLine(error.ErrorMessage);
}
}
ThrowBadRequest(messageBuilder.ToString());
}
}
protected void ThrowBadRequest(string message)
{
var responseMessage = new HttpResponseMessage(HttpStatusCode.BadRequest)
{
Content = new StringContent(message)
};
throw new HttpResponseException(responseMessage);
}
So here is what the comment creation method may end up looking like after applying the pattern a twice.
public async Task<Comment> Post(CommentCreateDto commentCreateDto)
{
ThrowBadRequestIfModelStateInvalid();
Post post = await PostService.GetAsync(commentCreateDto.PostId);
ThrowNotFoundIfNull(post);
//create the comment in the database
return comment;
}
First we check if the data is valid (max length and things like that), then we check if the comment is attached to a valid post and throw 404 if the post cannot be found. Then we create the comment and return it back to the caller. Note that this code lacks actual if statements. All the branches are encapsulated in the validation methods. We are not repeating the checks and thus avoiding code duplication.
While currently I am not aware of any downsides of this pattern in the future using it to check for nulls might be problematic. If C# introduces nullability tracking it will probably not be aware that the ThrowNotFoundIfNull method checked for null and will therefore force a null check again. One solution would be to implement the method like this
protected T ThrowNotFoundIfNull<T>(T? o)
{
return o ?? throw new HttpResposneException(…);
}
and then use it like this
post = ThrowNotFoundIfNull(post); //the compiler is now aware that the value is non-nullable
I am not sure if this particular generic implementation will be possible with what C# will implement but I hope there will be a solution to keep using the pattern in this downgraded way. Ideally the compiler will be aware of the check even without reassignment. I think this is possible to do with Code Contracts and postconditions but I think the proposal for C#’s nullability tracking is much less powerful than what Code Contracts can do. In any case the pattern will still be useful just like today in all other situations.