Code Smells, Primitive Obsession– Bloaters: Day 6

[Javier Arroyo]

This article is Day 6 and last in the Bloaters category 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

1. Code Smells, Refactoring Comments– Bloaters: Day 1
2. Code Smells, Refactoring Long Functions– Bloaters: Day 2
3. Code Smells, Refactoring Long Parameter Lists– Bloaters: Day 3
4. Code Smells, Refactoring Data Clumps Including Parameter Lists– Bloaters: Day 4
5. Code Smells, Refactoring Long Script Includes– Bloaters: Day 5

Day 6 - Refactoring Primitive Obsession

Primitive Obsession is the use of multiple variables to host data instead of using a smaller Script Include. 

Examples of primitive obsession

• Date rages
• user credentials
• clumps of related variables
• some constants

Solution

Refactoring is exactly the same as refactoring Long Parameter Lists or Data Clumps. Extract the primitives into a Script Include of its own, then replacing the use of primitives with calls to the new SI.

Why Primitive Obsession smells

• leads to code duplication
• Rigid/frigile
• can't be shared
• lack of structure/organization
• must maintain multiple unrelated locations
• added cluter

Example of a Primitive Obsession

this.startDate = new GlideDate();
this.startDate.setValue(startDate);
this.endDate = new GlideDate();
this.endDate.setValue(endDate);

this.noOfDays = GlideDateTime.subtract(this.startDate, this.endDate)
    .getDayPart() + 1;

The code fragment was extracted from an OOTB SI. 

My thoughts quickly become

• Can a more logical structure be introduced
• Can this be replaced to what is happening rather than how it's happening
• Can the relationship be made explicit instead of logical (It must be reason that startDate and endDate are somehow related)
• Is this duplicated else where
• how easily can additional functionality be added and how will it impact structure, clutter, size and ease to read
• Is anyone outside this SI looking at these variables and how will the change impact callers (how involved a refactoring is this going to be)

                                                                                                                                                                Example of Solution

Extract related variables into a new Script Include. Replace existing functionality with updated functionality in new SI. Add new functionality.

function DateCompare (startingDate, endingDate) {

    var startDate;
    var endDate;
    var numberOfDaySpan;

    initialize();

 return {
        span: span,
        endDate: getEndDate,
        startDate: getStartDate,
        setEndDate: setEndDate,
        setStartDate: setStartDate,

    };

    function setStartDate (value) {
        startDate = new GlideDate();
        startDate.setValue(value);

    }

    function getStartDate () {
        return startDate;

    }

    function setEndDate (value) {
        endDate = new GlideDate();
        endDate.setValue(value);

    }

    function getEndDate () {
        return endDate;

    }

    function setDateDifference () {
        numberOfDaySpan = GlideDateTime.subtract(startDate, endDate);

    }

    function span() {
        return numberOfDaySpan || GlideDateTime.subtract(this.startDate, this.endDate)
            .getDayPart() + 1;

    }

    function assertPrecondtions () {
        //determine how to fail (fast/safe)


    }

    function initialize () {
        assertPrecondtions();

        setStartDate(startingDate);
        setEndDate(endingDate);
        setDateDifference(startDate, endingDate);
    }

}

Above is the new demo Script Include, including some extra functionality as example of how to further the functionality with assertions. This class will now contain all common functionality expected from when comparing dates. While it looks like lot of work for just a few lines, the benefit is flexibility and reduced work each time data comparisons are needed. (In a prod version, I would prefer to pass parameters to each function rather than maintaining states within the SI that create hard dependencies)

The original code is then replaced with:

//from
this.startDate = new GlideDate();
this.startDate.setValue(startDate);
this.endDate = new GlideDate();
this.endDate.setValue(endDate);

//to
this.dateCompare = DateCompare('someDateString', 'someDateString2');

//wherever this.noOfDays ia used, it gets replaced with

this.dateCompare.span()  //my preference would be dateCompare.span(day1, day2); this are really is just demo code

To finalize the refactoring of primitive obsession, replace calls to old local primitives to function calls found in this.dateCompare. The relationship between primitives becomes explicit, the behavior understood at-a-glance, and the introduction of one stop shopping introduced which to call, update or test DateComparable functionality. 

If startDate and endDate were indeed being used as part of a public API, then those would still be exposed yet point to this.dateCompare.startDate(), etc.

The next category of code smells will talk about Abusers.