Code Smells, Refactoring Data Clumps Including Parameter Lists– Bloaters: Day 4

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 4 - Refactoring Data Clumps in Parameter lists or in-line code

A data clump is related data that is group together, be it in function arguments or throughout a codebase.

Examples of data clumps:

User credentials (password, username)
eBonding values (correlation_id, correlation_display)
Address (street, city, state, country)

Solution

The approach is precisely the same as that introduced in Code Smells, Refactoring Long Parameter Lists– Bloaters: Day 3 except that it deals with groups of related data.

Extract the logic into an Object of its own, isolating the units of work then, replace the clumps of data or related parameters with a function call. This will consolidate code into logical parts making more manageable.

Why they smell

An update to the functionality will require a search in the entire codebase to update similar functionality
Duplicates data
Leads to larger code surface in which bugs can hide
Increases entry point clutter
Order must be remembered
Disorganized code

Example

//Prameter list example
setEbonding(responseBody, correlationId, correlationValue, glideRecord);

//Data Clump in-line code example
var body = JSON.parse(response.getRequestBody());
var correlationId = body.value;
var correlationDisplay = body.key;

current.setValue('correlation_id', correlationId);
current.setValue('correlation_display', correlationDisplay);

current.update();

The code above can be more easily managed by clumping related items into an object of it's own.
Consider that in the case of the in-line data clump above, the code will need to be repeated nearly identically each time a correlation is needed. In the parameter list case, two of those items are so related that it one must live with the other, and we have to account for which is goes where in the parameter order. This will undoubtedly cause function setEbonding to account for a disproportionate amount of functionality; behavior that it should know nothing about. Like, what are the requirements for setting correlation values.

Worthy of note is that mistakes happen and integrations may not account for existing correlation values, overwriting them. By putting correlation behavior into one object there will be a robust and single entry point for all correlation behavior. The outcome is drier and single purpose code.

Solution example

Collect the logic and put it into a class of its own. Pass it a correlationOptions object that describes metadata needed to search the response; AND also account for the possibility that there can be existing correlation values.

Correlation(correlationOptions)
    .extractFromResponseBody(responseBody)
    .setValues(glideRecord)
    .save();

Having the functionality in a single place creates a great deal of flexibility. It reduces the amount of work other functions need to do, localizing logical groupings neatly. Anytime something needs to be addressed with correlation, there is a single place to look: Correlation object. No more need to hunt around hoping it's not in Workflow Activities, Business rules, UI Actions, Script Includes, Script Actions, transform maps....

When the moment comes that Correlation just doesn't have that one edge case solution, it can be extended with other methods or a map function that will take accept another function and execute it withing the context of the Correlation Object. For example, look for correlation values not from a responseBody but from a GlideRecord.

Correlation(correlationsOption)
   .sourceRecord(glideRecord)
   .transferToTarget(anotherGlideRecord)
   .save();

//or

Correlation(correlationsOption)
   .sourceRecord(glideRecord)
   .map(massageExistingCorrelationValuesFunction)//accepts function to do something unique
   .transferToTarget(anotherGlideRecord)
   .save();

Data Clumping is a great offender of clean code. It keeps mounding and mounding each time it is used. Not being diligent about maintaining a clean structure will quickly build an unmanageable code base.

Code can become a lot cleaner by moving items into logical units and replacing their usage with a function call.