Clean code with defensive programming technique

A lot of bugs is introduced because software developers often make assumptions about the input. I will show you a technique that will make your code more robust and easier to maintain.

The customer wants a function that can adjust product prices with a percentage value in the range 1%-30%.

Check this function:

/// <summary>
/// Adjusts product prices
/// </summary>
/// <param name="products">Array of products</param>
/// <param name="percentage">The price adjustment percentage (valid range = 1 to 30)</param>
public void AdjustPrice(IEnumerable<Product> products, double percentage)
{
    foreach (var product in products)
        product.Price *= (1d + percentage / 100d);
}

There are some problems with this code. First, the developer assumes that the input will always be correct - that the caller will always provide an array of products and a percentage within the valid range.

This is not good, we have to create a new version with input-parameter checking:

public void AdjustPrice(IEnumerable<Product> products, double percentage)
{
    if (products != null)
    {
        if (percentage >= 1d && percentage <= 30d)
        {
            foreach (var product in products)
            {
                product.Price *= (1d + percentage / 100d);
            }
        }
    }
}

Better this time, but there are still problems. What if the array of products contain null-references? Lets fix this:

public void AdjustPrice(IEnumerable<Product> products, double percentage)
{
    if (products != null)
    {
        if (percentage >= 1d && percentage <= 30d)
        {
            foreach (var product in products)
            {
                if (product != null)
                {
                    product.Price *= (1d + percentage / 100d);
                }
            }
        }
    }
}

Now, this looks robust, right? Yes, it's way better than the first version. But it isn't easy to read. And it isn't obvious how bad input should be handled. Personally, I prefer a more explicit checking strategy that eventually leads to less code indentation  (and better identification of how exceptions should be handled). Be specific about what parameters your function can't handle instead of nesting logical blocks of what conditions it can handle(!):

public void AdjustPrice(IEnumerable<Product> products, double percentage)
{
    if (products == null)
        throw new ArgumentNullException("products");
 
    if (percentage < 1d)
        throw new ArgumentException("Percentage can't be less than 1", "percentage");
 
    if (percentage > 30d)
        throw new ArgumentException("Percentage can't be higher than 30", "percentage");
 
    // It is now safe to iterate products and adjust price
 
    foreach(var product in products)
    {
        if (product == null)
            continue;       // Or throw exception if that is preffered
 
        // It is now safe to access the product reference and adjust its price
        product.Price *= (1d + percentage / 100d);
    }
}

Think of every iteration in the foreach loop as a call to an "inner function". Start the loop with loop-parameter checking the same way you check method-parameters and use the continue keyword to skip to the next item if it contains bad iteration-input. Or you could throw an exception - its all up to you and how you define your method "interface".

This is how it should look like. Minimal code indentation (nested logical blocks). Some few extra lines - true - but this is way easier to read and maintain.