C# - making null check fluent

Let's have the following piece of code, taken from a real codebase. Its main purpose is to find a person from a list of persons.  The excerpt was slightly simplified to make it shorter and comprehensive for the purpose of this article as in a real application, the search would be done using the Entity Framework and IQueryable and it would be a little bit complicated with more wheres, includes, and others lambda expressions).
using System;
using System.Collections.Generic;
using System.Linq;
 
namespace FluentSyntax
{
    class Person
    {
        public string FirstName { get; set; }
 
        public string LastName { get; set; }
    }
 
    class Program
    {
        private static List persons = new List
        {
            new Person {FirstName = "Joe", LastName ="Black" },
            new Person {FirstName = "Bill", LastName ="House"}
        };
 
        static void Main(string[] args)
        {
            var personsToFind = new[] { "Bla", "Mar", "Dow", "H" };
 
            foreach(var personToFind in personsToFind)
            {
                try
                {
                    Console.WriteLine($"{personToFind}: {FindPerson(personToFind).LastName}");
                }
                catch (Exception ex)
                {
                    Console.WriteLine($"{ personToFind}: {ex.Message}");
                }
            }
        }
  
        private static Person FindPerson(string lastNameStartsWith)
        {
            var person =
                persons
                    .Where(p => p.LastName.StartsWith(lastNameStartsWith))
                    .FirstOrDefault();
 
            if (person == null)
            {
                throw new Exception("Person not found.");
            }
 
            return person;
        }
    }
}

Issue

The FindPerson method contains a null check occupying four lines. Any unnecessary code lines make our code harder to read and understand. In real codebase, such a code structure is repeated over and over many times in the real codebase, once to check person, somewhere else to check cards, registered cars and so on. So it would be desirable to make this part shorter or more compact.

Just one line

First of all, let's reduce the number of lines.  To achieve that, the following extension method was defined:
public static class Extensions
{
    public static void ThrowIfNull<texception>(
        this object value,
        string message) where TException : Exception
    {
       if (value == null)
       {
           ThrowException<texception>(message);
       }
    }

    private static void ThrowException<exception>(string message) where TException : Exception
    {
        object[] argValue;

        if (typeof(TException).Equals(typeof(ArgumentNullException))
            ||
            typeof(TException).Equals(typeof(ArgumentOutOfRangeException)))
        {
            argValue = new object[] { null, message };
        }
        else
        {
            argValue = new object[] { message };
        }

        throw (TException)Activator.CreateInstance(typeof(TException), argValue);
    }
}

Using this extension method, the find a person code can be changed as follows:
private static Person FindPerson(string lastNameStartsWith)
{
    var person =
        persons
            .Where(p => p.LastName.StartsWith(lastNameStartsWith))
            .FirstOrDefault();

    person.ThrowIfNull<Exception>("Person not found");

    return person;
}

Undoubtedly, it is better now as there is only one additional line in the code. But wouldn't it be better to make it a part of the lambda search expression? In other words, add support for the fluent syntax?

Make it fluent

Let's explore this idea by changing the ThrowIfNull extension method:
public static TObject ThrowIfNull<TException, TObject>(
    this TObject value,
    string message) where TException : Exception
{
    if (value == null)
    {
        ThrowException<TException>(message);
    }

    return value;
}

The method to find a person  must be updated to:
private static Person FindPerson(string lastNameStartsWith)
{
    var person =
        persons
        .Where(p => p.LastName.StartsWith(lastNameStartsWith))
        .FirstOrDefault()
        .ThrowIfNull<Exception>("Person not found");

    return person;
}

The code is shorter now, which is good, however, it requires the coder to specify the return type in the code(<Exception, Person>). It would be desirable to be able to avoid it in the code.
To achieve that, the method should be split into two methods, one of them checking for null, the second one to create an exception: Here are these two new methods:
public static Action Throw<texception>(string message) where TException : Exception
{
    return () => ThrowException<texception>(message);
}

public static TObject IfNull<tobject>(this TObject value, Action action)
{
    if (value == null && action != null)
    {
        action();
    }

    return value;
}

Once implemented and after adding the following directive at the top of the code file after the usings:

using static FluentSyntax.Extensions;

we can change the code to:
private static Person FindPerson(string lastNameStartsWith)
{
    var person =
        persons
            .Where(p => p.LastName.StartsWith(lastNameStartsWith))
            .FirstOrDefault()
            .IfNull(Throw<Exception>("Person not found"));

    return person;
}

Conclusion

What was wrong with the first implementation? Remember the single responsibility principle - the method should be responsible for a single part of the functionality (SRP, part of the SOLID rules).
This rule was broken by the original method as it actually did two things - check for null and creating/throwing an exception.
It is fixed in the final implementation by moving the functionality into two independent methods. The first one checks for null and invokes an unspecified action, the second one provides the actual explicit action.

P.S: the method can be simplified more:
private static Person FindPerson(string lastNameStartsWith)
{
    return
        persons
            .FirstOrDefault(p => p.LastName.StartsWith(lastNameStartsWith))
            .IfNull(Throw<Exception>("Person not found"));
}

Note: never throw a general common exception, use the derived, specialized exception like the NullReferenceException and similar.

No comments:

Post a Comment