Code Smells, Refactoring Long Parameter Lists – Bloaters: Day 3

Javier Arroyo · ‎04-03-2020

This article is Day 3 of a multi-post series about refactoring Code Smells. Code Smells are characteristics that can negatively impact code; ranging from hindered read-ability, clutter, unscale-able, even outdated programming practices.

Refactoring practices will be mostly, if not all, from the Imperative/OO perspective as ServiceNow OOTB code is all written as such. I will try to be brief and attempt to use language features specific to JavaScript that may differ from those used by classically trained coders.

Previous series articles

Day 1 - refactoring comments.

The strategy is pretty straight forward: long parameter lists should become an Object.

Polluted entry point
must remember parameter order
indicative of a method that does too much
not clear if any parameters are optional
not apparent what the parameters mean
ambiguous
...

In JavaScript the key number of parameters to remember is 1. Pass one and only one parameter to a function; however, two are perfectly fine if a must. The less parameters a function accepts, the more concise the function will be, the easier, more basic it will read.

Example:

sendManagerEmail(true, false, 'anemail@that.com', null);

It's difficult to tell what each parameter represents. what are the booleans, why is email third and why is the fourth parameter null? At this point one wonders how the body of the method looks given the entry point. This should readily hint that the code is not as simple as it can be. The function will need to be opened up to identify parameter order, their meaning... as well as having to figure out how to construct the value for each parameter.

Solutions:

1. Use an object.

This is pretty straight forward, maybe not necessarily the most clean of methods but it becomes clearer what each value stands for and, is readily "refactorable" in a variety of ways.

var managerEmailOptions = {
   ccSubourdiantes: true,
   sendDuringWorkHours: false,
   from: 'anemail@that.com',
   notes: null
};

sendManagerEmail(managerEmailOptions);

From looking at managerEmailOptions parameter, it becomes obvious what each parameter represents. In addition, parameter order no longer matters, even skipping parameters all together is possible. Now it has become impossible not to know what each parameter means. It's got it's downsides but it at least becomes easier to understand their potential use.

2. Use a Builder

A builder is a class that uses set methods to instantiate properties. Think of something like GlideRecord.addQuery, except using set rather than add as a method prefix.

var emailOptions = ManagerEmailOptions()
  .setCanCCSubourdinates(true)
  .setCanSendDuringWorkHours(false)
  .setFromEmail('anemail@that.com')
  .setNotes('some notes')  //or not use this call at all
  .build();

sendManagerEmail(emailOptions);

The builder object does a few things: it makes the code easier to understand, removes ambiguity as to exactly what is being set, and more importantly, begins to shed light into how the parameter list can become grow into a class of its own. Leading to more manageable code. It even isolates logic into one and only one place. Developers now don't have to come up with their own logic spread across the codebase each time there is a need to call sendManagerEmail

Once having updated the parameter list, the body of the function will have to be adjusted accordingly.

The builder would look something like below, where individual functions create a location to add any necessary logic.

function ManagerEMailOptions (value) {
   
    initalize();

    return {
        setCanCCSubourdinates: setCanCCSubourdinates,
        setCanSendDuringWorkHours: setCanSendDuringWorkHours,
        setFromEmail: setFromEmail,
        setNotes: setNotes,
        build: build
    };

    function initalize () {
        if (value === null || value === undefined) {
            value = {};
        }  
    }

    function setCanSendDuringWorkHours (canSend) {
       value.cendDuringWorkingHours = canSend;
       
       return ManagerEMailOptions(value);
    }

    function setCanCCSubourdinates (canCC) {
        value.canCCSubourdinates = canCC;

        return ManagerEMailOptions(value);
    }

    function setFromEmail (emailAddress) {
        value.fromEmail = emailAddress;

        return ManagerEMailOptions(value);
    }

    function setNotes (notes) {
        value.notes = notes;

        return ManagerEMailOptions(value);
    }

    function build () {
        return value;

    }

}

Keep in mind that the goal is to express your thoughts in a clear and apparent manner to the next person that needs to read your code. It's really not as much about how terse, or clever code is but, how easily it is to both read the code AND understand it. If the reader has to jump around to fully grasp an entry point function... well, we all hope it's because the developers is a beginner rather than our code being smelly.