Andy Crouch - Code, Technology & Obfuscation ...

Observations About If Statements

Photo: Raul Petri - unsplash.com

In this post, I want to highlight a couple of observations about the use of conditional if statements. All languages have a variety of the humble if statement. Programs would not be able to function without them and the logic they provide. Some might say they are the basis of very early AI but I digress.

The basic outline of an if statement is as follows:

if("a specific condition equates to true")
{
    DoSomething();
}
else
{
    DoSomethingElse();
}

This say’s that if the tested condition is true then execute the DoSomething() method. If not execute the DoSomethingElse() method. All pretty straightforward.

But, there are some obvious issues that we should look out for. We do not want to make our code unreadable or hard to understand.

The first observation relates to when many conditions are tested in one go which makes reading the if line a little harder. If reading the conditional test logic is hard then reversing it to understand how you get to the else is even harder.

if((person.Age > 18 && person.Age < 25) && 
   group.Name == "Test Group 1" || group.Name == "Test Group 5")
{
    DoSomething()
}
else
{ 
    DoSomethingElse(); 
}

Vs

var isPersonInTargetAgeRange = person.Age > 18 && person.Age < 25; 
var isATargetGroup = "Test Group 1" || group.Name == "Test Group 5";

if(isPersonInTargetAgeRange && isATargetGroup)
{
    DoSomething()
}
else
{ 
    DoSomethingElse(); 
}

Creating well-named variables and using them in the test condition makes it much easier to read. It is also easier to understand the reverse logic when determining how the else block is reached.

The next problem is the use of deep nested if statements. I have seen a lot of examples of this lately. Code such as:

public string Read()
{
    if(Path.DirectoryExists(_testDirectory))
    {
        if(Path.FileExists(_testFilePath))
        {
            if(!File.IsReadOnly(_testFilePath))
            {
                var file = File.Open(_testFilePath);
                if(file != null)
                {
                    file.Read();
                }
            }
        }
    }
}

The more conditions that are added add a further level of nesting. Code usually ends up like this due to extending requirements. Deep nesting makes it hard to understand the logic that leads to the actual execution of a piece of code. It becomes a lot harder when you have else clauses at multiple levels as well.

public string Read()
{
    if(!Path.DirectoryExists(_testDirectory))
        return;

    var testFileExists = Path.FileExists(_testFilePath);
    var testFileIsReadOnly = File.IsReadOnly(_testFilePath);

    if(!testFileExists || !testFileIsReadOnly)
	return;

    var file = File.Open(_testFilePath);

    if(file != null)
        file.Read();
}

The final issue I want to cover is the use of an if statement to wrap the body of a method. This is another approach which I have seen a lot lately. Something like:

public void AMethod(bool aCondition)
{
    if(aCondition != true)
    {
        LogCondition();
    }
    else
    {
        OpenConnection();
        RequestData();
        ProcessData();
        CloseConnection();
    }
}

Why is the else used? Why not just add a return in the if block and remove the else? The reverse of this is even more strange:

public void AMethod(bool aCondition)
{
    if(aCondition == true)
    {
        OpenConnection();
        RequestData();
        ProcessData();
        CloseConnection();
    }
    else
    {
        LogCondition();
    }
}

Why? Why would you write this? You should look to exit your functions and methods at the earliest opportunity. This is why approaches such as parameter checking are available in most languages. There are patterns that mean you can check and exit out if you get a null or unexpected value. Wrapping your method body like either of these examples is a clear signal you need to refactor your code.

public void AMethod(bool aCondition)
{
    if(aCondition != true)
        LogCondition();

    OpenConnection();
    RequestData();
    ProcessData();
    CloseConnection();
}

if statements are a core part of most programming languages but you need to ensure their use does not result hard to understand code. If you find yourself nesting if statements or if you find that your test conditions span multiple lines then you almost certainly need to refactor your code.

If you have any thoughts about this post then please contact me via twitter or email.