Best Practices

Best Practices for Functions

This article will provide guidance on how to improve the readability, scalability, and secure functions.

When Should a Function be Created

A function is a couple of statements that make up a single task. Functions are the backbone of all modern languages. They are intended to be simple and easy to read. The function name, as described in the previous blog Naming Conventions, should also be clearly defined to explain the task the function will do. Now that we know what a function is, it's important to undertand when it should be created.

Avoid Duplication

The most straightforward answer to when functions need to be created is to avoid code duplication. Code is a liablity and it's important that anytime code is repeated, replace it with a function call. Some developers may state that they're doing this for performance, but that is not an excuse. Writing legible code is more important. If performance is indeed a requirement, mark the functions as inline. Most modern languages support that.

The principles of avoiding duplication are:

  • Don't repeat yourself
  • Code is a liability
  • Less is more

Good

std::string getStringFromJson(const Json::Value& value)
{
    Json::StreamWriterBuilder           builder;
    std::ostringstream                  stream;
    std::unique_ptr<Json::StreamWriter> writer;
     
    // set builder defaults
    builder["commentStyle"]             = "None";
    builder["indentation"]              = "";
    builder["enableYAMLCompatibility"]  = false;
    builder["dropNullPlaceholders"]     = false;
    builder["useSpecialFloats"]         = false;
    builder["precision"]                = 17;
     
    writer = std::unique_ptr<Json::StreamWriter>(builder.newStreamWriter());
    writer->write(value, &stream);
    return stream.str();
}
 
std::string getPersonalInformation()
{
    Json::Value value = this->getPersonInformation();
    return getStringFromJson(value);
}
 
std::string getBusinessInformation()
{
    Json::Value value = this->getPersonInformation();
    return getStringFromJson(value);
}

Bad

std::string getPersonalInformation()
{
    Json::Value& value                  = this->getPersonInformation();
 
    Json::StreamWriterBuilder           builder;
    std::ostringstream                  stream;
    std::unique_ptr<Json::StreamWriter> writer;
     
    // set builder defaults
    builder["commentStyle"]             = "None";
    builder["indentation"]              = "";
    builder["enableYAMLCompatibility"]  = false;
    builder["dropNullPlaceholders"]     = false;
    builder["useSpecialFloats"]         = false;
    builder["precision"]                = 17;
     
    writer = std::unique_ptr<Json::StreamWriter>(builder.newStreamWriter());
    writer->write(value, &stream);
    return stream.str();
}
 
std::string getBusinessInformation()
{
    Json::Value& value                  = this->getPersonInformation();
 
    Json::StreamWriterBuilder           builder;
    std::ostringstream                  stream;
    std::unique_ptr<Json::StreamWriter> writer;
     
    // set builder defaults
    builder["commentStyle"]             = "None";
    builder["indentation"]              = "";
    builder["enableYAMLCompatibility"]  = false;
    builder["dropNullPlaceholders"]     = false;
    builder["useSpecialFloats"]         = false;
    builder["precision"]                = 17;
     
    writer = std::unique_ptr<Json::StreamWriter>(builder.newStreamWriter());
    writer->write(value, &stream);
    return stream.str();
}

Avoid Too Much Indentation In Function

Indentation is a good indicator on when a function needs to be split into one main function and sub functions. It shows that the function has more distincts path and can lead to more complexity. This code becomes more prone to an arrow shape code. This hurts readability, hinders testing and increase the likelihood of bugs.

Looking at the examples below, you can notice that the good example is more readable and is easier to unit test. The bad example makes reading so much harder. Hard to read code is not only bad for code review, it also introduces bugs and makes debugging harder.

Good

if (cond1)
{
    doStuff1()
}
else
{
    doStuff2()
}

Bad

if (cond1)
{
    if (cond2)
    {
        if (cond3)
        {
            if  (cond4)
            {
                do
                a
                complicated
                thing
            }
            else
            {
                do
                some
                more
                complicated
                things
            }
        else
        {
            do
            a
            complicated
            thing
        }
    else
    {
        do
        something
    }
else
{
    do
    return
    error
}

Return Early

Returning early also improves readability and promotes testability. As you can notice in the following bad example, the code is prone to the arrow structure which makes reading it hard and hinders testing.

bool function1()
{
    if (cond1 == true) 
    {
        if (cond2  == true)
        {
            if (cond3 == true)
            {
                if (cond4 == true)
                {
                    return doStuff();
                }
            }
        }
    }
    return false;
}

By returning early, you can avoid that by keeping the structure flat. The following example is more readable and can be debugged more easily.

bool function1() 
{
    if (cond1 == false)
    {
        return false;
    }
    if (cond2 == false)
    {
        return false;
    }
    if (cond3 == false)
    {
        return false;
    }
    if (cond4 == false)
    {
        return false;
    }
    return doStuff();
}

It is important to note that returning early can have its own caveats. For example check out the following code:

bool function1() 
{
    char* arr = (char*)malloc(100);
    populateArray(arr);

    if (checkCondition1(arr) == false) 
    {
        free(arr);
        return false;
    }

    char* arr2 = (char*)malloc(100);
    populateArray(arr2);

    if (compare(arr1,arr2) == false) 
    {
        free(arr1);
        free(arr2);
        return false;
    }

    free(arr1);
    free(arr2);
    arr1 = NULL;
    arr2 = NULL;
    
    if (cond3 == false) 
    {
        return false;
    }

    if (cond4 == false) 
    {
        return false;
    }

    return doStuff();
}

If you can notice from the code above, we were able to prevent memory leaks by freeing arr1 and arr2 on the first and second condition before returning. However, as code becomes more complex, memory leaks can slip through. A better concept of returning early is by using a do {} while(0) trick.

bool function1() 
{
    char* arr1 = NULL;
    char* arr2 = NULL;
    
    do
    {

        arr1 = (char*)malloc(100);
        populateArray(arr1);

        if (checkCondition1(arr1) == false) 
        {
            break;
        }

        arr2 = (char*)malloc(100);
        populateArray(arr2);

        if (compare(arr1,arr2) == false) 
        {
            break;
        }

        free(arr1);
        free(arr2);
        arr1 = NULL;
        arr2 = NULL;
        
        if (cond3 == false) 
        {
            break;
        }

        if (cond4 == false) 
        {
            break;
        }

    }
    while(0);
    
    if (arr1 != NULL) 
    {
        free(arr1);
        arr1 = NULL;
    }

    if (arr2 != NULL) 
    {
        free(arr2);
        arr2 = NULL;
    }

    return doStuff();
}

The above concept retains the return early rule by using break. However, in this case it just returns to the end of the function to do some cleanup.

Fail Fast

Similar to returning early, failing fast can help readability and testing.

Good

void registerUser(std::string username, std::string password) {
    if (isNullOrWhitespace(username) == true)
    {
        throw new ArgumentException("Username is invalid.");
    }  

    if (isNullOrWhitespace(password) == true)
    {
        throw new ArgumentException("Password is invalid.");
    }  
    
    // register user here
}

Bad

void registerUser(std::string username, std::string password) {
    if (isNullOrWhitespace(username) == false) 
    {
        if (isNullOrWhitespace(password) == false) 
        {
            // register user here
        }
        else
        {
            throw new ArgumentException("Password is invalid.");
        }
    }
    else 
    {
        throw new ArgumentException("Username is invalid.");
    }
}

Convey Intent

Another reason to create new functions is to convey intent. This can make the code more readable and way easier to debug. Think about functions as headings and subheadings of a chapter.

Good

bool isValidFileRequest(...)
{
    std::vector<std::string> validExtensions = { "mp4", "mpg", "avi" };     
    bool validFileType = (std::find(validExtensions.begin(),
                                    validExtensions.end(),
                                    fileExtension)) != validExtensions.end();


    bool userIsAllowedToViewFile = (isAdmin || isActiveFile);

    return (validFileType && userIsAllowedToViewFile);
}

void consumeFile(...)
{
    if (isValidFileRequest(...) == false)
    {
        throw new Exception("Invalid file request.");
    }
    
    // consume file here
}

Bad

void consumeFile(...)
{
    if ((fileExtension == "mp4" || 
        fileExtension == "mpg" || 
        fileExtension == "avi") &&
        (isAdmin || isActiveFile)) 
    {
        // do request
    }
    else
    {
        throw new Exception("Invalid file request.");
    }
}

Functions Should Do One Task Only

The idea of a function is that it should convey a single task. This helps in:

  • Aiding the reader
  • Promotes reuse
  • Eases naming and testing
  • Avoids side effects

Minimize Number of Parameters

Functions should have minimal number of parameters. Developers should thrive for 0 - 2 parameters. This makes the function easier to understand, test, and helps assure that the function does on thing. Having a lot of parameters is a good indicator that these parameters belong to a user object or a class.

Good

void saveUser(User user)
{
    // save user here.
}

Bad

void saveUser(std::string firstName, std::string lastName, std::string email, std::string phoneNumber)
{
    // save user here.
}

Be Careful of Flag Arguments

It's a good hint that functions do more than one task if it has flag argument. This means that the function may do more than one task.

Good

void saveUser(User user)
{
    // save user here.
}

void emailUser(User user)
{
    // email user here.
}

Bad

void saveUser(User user, bool emailUser)
{
    // save user here.

    if (emailUser)
    {
        // email user
    }
}

Handle Exceptions Based on Criteria

When writing a function beware on how to handle errors. Errors can be classified as Unrecoverable, Recoverable, and Ignorable.

Unrecoverable

Unrecoverable errors such as null reference, file not found, access denied, etc should result in a thrown exception. Not doing so may cause bad behaviors as app evolves.

Bad

try
{
    saveUser(); // throws invalid username exception
}
catch (Exception e)
{
    logError(e);
}
emailUser();

In the above example, the code misbehaves because email user is called after saving failed. This can lead to behavior issue. Allow the exception to be thrown and let the caller handle the exception if needed.

Good

saveUser(); // throws invalid username exception
emailUser();

Recoverable

Recoverable errors are errors that can be recovered from by simply retrying the request. A network request, for instance, don't have to throw an exception because it may recover after retry. Please be aware that retries shouldn't be indefinite. Giving up on a retry will prevent the app from being stuck in an indefinite state.

Ignorable

Ignorable errors are rare. However, an ignorable error can be an error that do not effect the behavior of the app on the user. An example of that is an error in logging function. These errors can be safely ignored.

Conclusion

It is important how to define functions and understand when to use functions when developing your next application. In summary, functions should do one task and one task only. They should be clear and easy to read. This will help reduce bugs and make debugging easier.

Author image

About Rawad Hilal

Lead programmer with a track record of incorporating user and business requirements into cost-effective, secure and user-friendly solutions known for scalability and durability.
  • Orange County, California, USA