Reducing Boilerplate Code - Refactoring into a Utility Function

Javier Arroyo · ‎03-03-2020

This article follows along the lines of previous posts on code architecture and readable code. It will use refactoring to scale an existing function that sets journal and non journal field values given an Object literal coming from an HTTPRequest.body.bodyString.

As a refactoring practice, I will use a the rule of thumb that says: whenever you touch a piece of code, leave it in a nicer state that you found it. This is simply done to decrease the amount of time that it will take the next developer to read function behavior.

Problem:
Scripted web services have become fragile, complex, certainly duplicating by necessity boilerplate code. As a first pass, scale setting field values in a record from values found in HTTPRequest.body.bodyString

Solution:
1. Introduce an Object to handle setting field values.
2. Refactor web services to use new function

Original Function

function setFieldsToRecord (record, validFieldValues) {
    for (var fields in validFieldValues) {
        if (requestBody.hasOwnProperty(fields))
            record.getElement(fields)
            .getED()
            .getInternalType() == 'journal_input' ? record[fields] = requestBody[fields] : record.setValue(fields, requestBody[fields]);
    }

    if (requestBody.hasOwnProperty('assignment_group')) {
        record.setValue('assignment_group', getAssignmentGroup(requestBody['assignment_group']));
    }

    record.update();
    return record;
}

Given a record and a key/pair object with fields and field values, the function

loops through reach field,
checks if it’s a journal field or not,
uses the respective GlideRecord field type functionality to set the value
saves the record.

The goal here is to decrease the amount of time it takes to digest this function. Make it so that it reads like sentences on a book.

Note: this is no more than demo script showing how to scale and create more polite-to-the-reader code. At an atomic level, the script will look bigger but, it is at the macro level the difference is made. Libs or utility functions end up decreasing the overall size of the code base. So this Script Include will contain more surface lines than the original solution. It gives up lines to gain readability, abstraction, and single purpose functions.

Refactoring Process

The refactoring was pretty standard:

Reduce a larger function by extracting functionality until there is no longer something else that can be extracted
Make calls to if/ternary predicate functions giving each side of the condition its own function to read it as if a sentence on a book
Place the extracted functions into one Script Include to easily find them and do it in an OOish sort of solution.
Don’t leave around ambiguous magic strings or numbers
Each extracted method should be no longer than 4 lines long
Method names should identify the encapsulated functionality
Functionality must be the same (what the old code did, must be done by the new code)
Limit the use of comments by introducing function names to make up for the comments

Anticipated Implementation

A script include with a couple of public members and hiding away private members. The functions within the script include will be so tiny trying to reduce the time it takes to read behavior.

Refactored Function into Script Include

function GlideFieldSetter (record) {
    return {
        setFieldValues: setFieldValues,
        saveUpdates: saveUpdates
    }
    /********************************Public Members************************************************ */
    function setFieldValues (fields) {
        fields.forEach(setValues(fields));
    }
    function saveUpdates () {
        return record.update();

    }

    /*******************************Private Members************************************************ */

    function setValues (fields) {
        return function saveValue (field) {
            return isJournalInput(field) ?
                setJournalFieldValue(field, fields[field]) :
                setNonJournalFieldValue(field, fields[field]);
        }
    }
    function setNonJournalFieldValue (field, value) {
        record[field].setValue(value);
        return field;
    }
    function setJournalFieldValue (field, value) {
        record[field] = value;
        return field;
    }

    function isJournalInput (field) {
        var JOURNAL_FIELD_IDENTIFIER = 'journal_input';

        return record.getElement(field)
            .getED()
            .getInternalType == JOURNAL_FIELD_IDENTIFIER;
    }
}

Explanation

GlideFieldSetter exposes two functions:

  return {
        setFieldValues: setFieldValues,
        saveUpdates: saveUpdates
    }

They do precisely what they say. One sets field values, the other saves the updates caused by the field values

setFieldValues does one thing. It delegates saving field values to someone else so that it can be read obstruction free

    function setFieldValues (fields) {
        fields.forEach(setValues(fields));
    }

It passes the output of calling setValues(fields) to forEach. The result of setValues is another function named saveValue. This means that as value/pairs go through the function, the function saveValue will be called.

setValues, the private function called from pubilc funcion setFieldValues is a curried function of depth 2 that when executed, returns another function saveValue

    function setValues (fields) {

        return function saveValue (field) {

            return isJournalInput(field) ?
                setJournalFieldValue(field, fields[field]) :
                setNonJournalFieldValue(field, fields[field]);

        }
    }

setValues does nothing more than delegating its work to other functions in order to create readable code so that it looks more inline with idiomatic grammar. It works by partially applying a function during it's first execution during (fields.forEach(setValues(fields))). The resulting second function can then be used independently each time through the loop. The returned function saveValue (field) will now be able to check any field passed to it as isJournalInput or not. If it is, it delegates to another function setJournalFieldValue; otherwise, it calls setNonJournalFieldValue. Reading it now reads as if isJournalInput call setJournalFieldValue else setNonJournalFieldValue. Quite the dramatic reading comprehension when compared to the original condition:

Refactored condition

isJournalInput(field) ?
                setJournalFieldValue(field, fields[field]) :
                setNonJournalFieldValue(field, fields[field]);

vs

if (requestBody.hasOwnProperty(fields))

            record.getElement(fields)
            .getED()
            .getInternalType() == 'journal_input' ? record[fields] = requestBody[fields] : record.setValue(fields, requestBody[fields]);

the actual implementation of the if statement has been abstracted away and identified with isJournalInput

function isJournalInput (field) {
    var JOURNAL_FIELD_IDENTIFIER = 'journal_input';

    return record.getElement(field)
        .getED()
        .getInternalType == JOURNAL_FIELD_IDENTIFIER;

}

Lastly, the last of the two exposed public methods saves the record. saveUpdates does nothing more than saving the record. This is done to keep the process cohesive as if the same process. It can be eliminated if so desired.

    function saveUpdates () {
        return record.update();

    }

The final function call and new entry point from scripted web services

function setFieldsToRecord (record, fields) {
    var setter = GlideFieldSetter(record);
    setter.setFieldValues(fields);
    setter.saveUpdates();
    return record;

}

The final function call is much cleaner. It has reduced mental load down to reading the steps required to save field values. The new function can now replace, not only values being set from inbound requests but, anywhere there is a need to set GlideRecod values. This means a reduced size codebase in which to hide bugs. More importantly, the functionality can exist completely independently, therefore, testing will be much simpler.

As mindful developers, the next step is to for the next person that touches it, to make it cleaner, more scalable, etc.

Side by side comparison of original vs refactored

Original

function setFieldsToRecord (record, validFieldValues) {
    for (var fields in validFieldValues) {
        if (requestBody.hasOwnProperty(fields))
            record.getElement(fields)
            .getED()
            .getInternalType() == 'journal_input' ? 
                    record[fields] = requestBody[fields] :
                    record.setValue(fields, requestBody[fields]);
    }

    if (requestBody.hasOwnProperty('assignment_group')) {
        record.setValue('assignment_group', 
      getGroupMappingSysID(requestBody['assignment_group']));
    }

    record.update();
    return record;
}

Refactored

function setFieldsToRecord (record, fields) {
    var setter = GlideFieldSetter(record);
    setter.setFieldValues(fields);
    return setter.saveUpdates();

}

Think of code as being in layers of abstraction, like reading a thesis paper or literary article.
There is a subject, followed by some more of TLDR (too long to read) paragraph then, as the reader delves into the paragraphs, more details are revealed. When the reader is no longer interested in reading, he stops. For maintenance, this means that when a developer is asked what does what does setFieldToRecord do? he says, it sets field values then saves the updates. At this level of abstraction, there is no need to know more, only if the interest and need is there for implementation details, should one delve into a deeper into details.