Wednesday 27 July 2022

Code indentation reduction using Guard Clauses

Excessive indentation seems to be a common plague among programmers, regardless of your years of experience. Ever so often, I find myself getting caught in multiple levels of logic, and the inevitable nesting that follows, makes my code terribly hard to read. I should know; I've been called out plenty of times on this!

Take this example in JavaScript.

function f (x, y)
{
  if (x)
  {
    if (y)
    {
      return y;
    }
    else
    {
      return x;
    }
  }
}


To combat this, programmers use early exit techniques, and recently, I discovered there was actually a term for this: guard clauses.

That's right, decades after my first Hello World program, I'm still learning new stuff. Perhaps even stuff I should have learned a long time ago. But, as they say, better late than never.

Now, in the example above, we don't do anything if x is false. There's no Else clause. So what we do, is exit early.

function f (x, y)
{
  if(!x) return;
 
  //if (x)
  //{
    if (y)
    {
      return y;
    }
    else
    {
      return x;
    }
  //}
}


So the code now looks like this.
function f (x, y)
{
  if(!x) return;
 
  if (y)
  {
    return y;
  }
  else
  {
    return x;
  }
}


See what we did there? We cut to the chase. Now the code just has one less If clause taking up the cognitive load. But is that enough? Not at all; we can actually simplify this further.

function f (x, y)
{
  if(!x) return;
  if(!y) return x;

  //if (y)
  //{
    return y;
  //}
  //else
  //{
  //  return x;
  //}
}


The code now looks like this. What we did was make the code return x automatically if y is false. And then the code returns y by default, but that line will never be reached if x or y is false. And there! Now there are no further If clauses to deal with. Two levels of nesting have been removed, and the code looks cleaner already.
function f (x, y)
{
  if(!x) return;
  if(!y) return x;

  return y;
}


So, these are guard clauses. They conduct checks, and pre-emptively exit the code when appropriate, so that no further processing needs to be done. Other than the obvious computational savings though, this also serves to reduce the cognitive load on the reader of the code.

A guard clause is like security
ejecting you early.

Think of guard clauses as bouncers at a night club guarding against unruly patrons and ejecting them when necessary.

An actual working example

Now let's take a look at an actual back-end script in PHP. This was used to create a temporary CSV file, write to it using data based on the current day, and then send to an FTP address. Obviously, the FTP login and password has been fudged here, but the rest of it is authentic enough. $result in this case is an array of objects derived from executing a query to the database. From $result, we want to write to the CSV file all data that does not fall on a weekend, and the total has to be a positive number.

function deliverDailyFile($result)
{
    $conn_id = ftp_connect('111.222.333.444');
    $login_result = ftp_login($conn_id, 'tthunder', 'thund3r$trike!!!');

    if ($login_result)
    {
        $folder = ABSPATH . 'cron/01/transactions/daily/';
        $filename =  "transactions_" . date('Y_m_d') . ".csv";
        $fp = fopen($folder . $filename, 'w');

        if ($fp)
        {
            $counter = 0;
        
            foreach ($result as $r)
            {
                if ($r->dayOfWeek != 0 && $r->dayOfWeek != 6)
                {
                    if ($r->total > 0)
                    {
                        $counter++;
                        fputcsv($fp, [$counter, $r->year, $r->month, $r->day, $r->num_transactions, $r->category, $r->total]);
                    }
                }
            }
        
            ftp_pasv($conn_id, true);
            $uploaded = ftp_put($conn_id, 'transactions/' . $filename, $folder . $filename, FTP_BINARY, 0);
            
            if ($uploaded)
            {
                unlink($folder . $filename);
            }
            else
            {
                return false;
            }

            fclose($fp);
        }
        else
        {
            return false;
        }
    }
    else
    {
        return false;
    }
    
    return true;
}


Now, for what amounts to a very simple script, there's too much indentation and it is hard to read. It would be a lot harder to read once the code inevitably gets extended or altered. To remove the first level of indentation, we introduce a guard clause to check for the value of $login_result. If it is false, exit early, returning false.
function deliverDailyFile($result)
{
    $conn_id = ftp_connect('111.222.333.444');
    $login_result = ftp_login($conn_id, 'tthunder', 'thund3r$trike!!!');

    if (!$login_result) return false;

    //if ($login_result)
    //{
        $folder = ABSPATH . 'cron/01/transactions/daily/';
        $filename =  "transactions_" . date('Y_m_d') . ".csv";
        $fp = fopen($folder . $filename, 'w');

        if ($fp)
        {
            $counter = 0;
        
            foreach ($result as $r)
            {
                if ($r->dayOfWeek != 0 && $r->dayOfWeek != 6)
                {
                    if ($r->total > 0)
                    {
                        $counter++;
                        fputcsv($fp, [$counter, $r->year, $r->month, $r->day, $r->num_transactions, $r->category, $r->total]);
                    }
                }
            }
        
            ftp_pasv($conn_id, true);
            $uploaded = ftp_put($conn_id, 'transactions/' . $filename, $folder . $filename, FTP_BINARY, 0);
            
            if ($uploaded)
            {
                unlink($folder . $filename);
            }
            else
            {
                return false;
            }

            fclose($fp);
        }
        else
        {
            return false;
        }
    //}
    //else
    //{
    //    return false;
    //}
    
    return true;
}


At this point, $login_result is definitely true if the code is still running. The next check is for $fp to be true.
function deliverDailyFile($result)
{
    $conn_id = ftp_connect('111.222.333.444');
    $login_result = ftp_login($conn_id, 'tthunder', 'thund3r$trike!!!');

    if (!$login_result) return false;

    $folder = ABSPATH . 'cron/01/transactions/daily/';
    $filename =  "transactions_" . date('Y_m_d') . ".csv";
    $fp = fopen($folder . $filename, 'w');

    if ($fp)
    {
        $counter = 0;

        foreach ($result as $r)
        {
            if ($r->dayOfWeek != 0 && $r->dayOfWeek != 6)
            {
                if ($r->total > 0)
                {
                    $counter++;
                    fputcsv($fp, [$counter, $r->year, $r->month, $r->day, $r->num_transactions, $r->category, $r->total]);
                }
            }
        }

        ftp_pasv($conn_id, true);
        $uploaded = ftp_put($conn_id, 'transactions/' . $filename, $folder . $filename, FTP_BINARY, 0);

        if ($uploaded)
        {
            unlink($folder . $filename);
        }
        else
        {
            return false;
        }

        fclose($fp);
    }
    else
    {
        return false;
    }
    
    return true;
}


Therefore, the next guard clause checks for the value of $fp, and exits early if false. Thus, we remove another level of indentation.
function deliverDailyFile($result)
{
    $conn_id = ftp_connect('111.222.333.444');
    $login_result = ftp_login($conn_id, 'tthunder', 'thund3r$trike!!!');

    if (!$login_result) return false;

    $folder = ABSPATH . 'cron/01/transactions/daily/';
    $filename =  "transactions_" . date('Y_m_d') . ".csv";
    $fp = fopen($folder . $filename, 'w');
    
    if (!$fp) return false;

    //if ($fp)
    //{
        $counter = 0;

        foreach ($result as $r)
        {
            if ($r->dayOfWeek != 0 && $r->dayOfWeek != 6)
            {
                if ($r->total > 0)
                {
                    $counter++;
                    fputcsv($fp, [$counter, $r->year, $r->month, $r->day, $r->num_transactions, $r->category, $r->total]);
                }
            }
        }

        ftp_pasv($conn_id, true);
        $uploaded = ftp_put($conn_id, 'transactions/' . $filename, $folder . $filename, FTP_BINARY, 0);

        if ($uploaded)
        {
            unlink($folder . $filename);
        }
        else
        {
            return false;
        }

        fclose($fp);
    //}
    //else
    //{
    //    return false;
    //}
    
    return true;
}


Can this be improved? Undoubtedly. There's too much indentation within the For-each loop.
function deliverDailyFile($result)
{
    $conn_id = ftp_connect('111.222.333.444');
    $login_result = ftp_login($conn_id, 'tthunder', 'thund3r$trike!!!');

    if (!$login_result) return false;

    $folder = ABSPATH . 'cron/01/transactions/daily/';
    $filename =  "transactions_" . date('Y_m_d') . ".csv";
    $fp = fopen($folder . $filename, 'w');
    
    if (!$fp) return false;

    $counter = 0;

    foreach ($result as $r)
    {
        if ($r->dayOfWeek != 0 && $r->dayOfWeek != 6)
        {
            if ($r->total > 0)
            {
                $counter++;
                fputcsv($fp, [$counter, $r->year, $r->month, $r->day, $r->num_transactions, $r->category, $r->total]);
            }
        }
    }

    ftp_pasv($conn_id, true);
    $uploaded = ftp_put($conn_id, 'transactions/' . $filename, $folder . $filename, FTP_BINARY, 0);

    if ($uploaded)
    {
        unlink($folder . $filename);
    }
    else
    {
        return false;
    }

    fclose($fp);
    
    return true;
}


So we add a guard clause that checks if $r->dayOfWeek falls on a weekend, and we use the continue statement if so.
foreach ($result as $r)
{
    if ($r->dayOfWeek == 0 || $r->dayOfWeek == 6) continue;

    //if ($r->dayOfWeek != 0 && $r->dayOfWeek != 6)
    //{
        if ($r->total > 0)
        {
            $counter++;
            fputcsv($fp, [$counter, $r->year, $r->month, $r->day, $r->num_transactions, $r->category, $r->total]);
        }
    //}
}


At this point, if the code is still running within the loop, $r->dayOfWeek is definitely a weekday. We can actually eliminate the last If block as well. It seems a little excessive, but sure, let's do it.
foreach ($result as $r)
{
    if ($r->dayOfWeek == 0 || $r->dayOfWeek == 6) continue;
    if ($r->total <= 0) continue;

    //if ($r->total > 0)
    //{
        $counter++;
        fputcsv($fp, [$counter, $r->year, $r->month, $r->day, $r->num_transactions, $r->category, $r->total]);
    //}
}


This is the final result. We have reduced it to one If-else block. This mostly works for cases where nothing much happens in the Else part of the If-else block, so be cautioned against overusing this.
function deliverDailyFile($result)
{
    $conn_id = ftp_connect('111.222.333.444');
    $login_result = ftp_login($conn_id, 'tthunder', 'thund3r$trike!!!');

    if (!$login_result) return false;

    $folder = ABSPATH . 'cron/01/transactions/daily/';
    $filename =  "transactions_" . date('Y_m_d') . ".csv";
    $fp = fopen($folder . $filename, 'w');
    
    if (!$fp) return false;

    $counter = 0;

    foreach ($result as $r)
    {
        if ($r->dayOfWeek == 0 || $r->dayOfWeek == 6) continue;
        if ($r->total <= 0) continue;
        
        $counter++;
        fputcsv($fp, [$counter, $r->year, $r->month, $r->day, $r->num_transactions, $r->category, $r->total]);
    }

    ftp_pasv($conn_id, true);
    $uploaded = ftp_put($conn_id, 'transactions/' . $filename, $folder . $filename, FTP_BINARY, 0);

    if ($uploaded)
    {
        unlink($folder . $filename);
    }
    else
    {
        return false;
    }

    fclose($fp);
    
    return true;
}


Conclusion

Code works whether or not guard clauses are utilized. But working code is the bare minimum. Code should be easy to read, and intent made clear as much as possible. Guard clauses accomplish this, and consequently, makes code easier to maintain.

if (endPost) return;
T___T

No comments:

Post a Comment