Code Smells, Temporary Fields – Object Oriented Abusers: Day 8

[Javier Arroyo]

This article is Day 8 and second article of the Object Oriented Abusers category in a multi-post series about refactoring Code Smells. Code Smells are characteristics that can negatively impact code; ranging from hindered read-ability, clutter, unscale-ability, even outdated programming practices.

Refactoring examples will be mostly, if not all, from the Imperative/OO perspective as ServiceNow OOTB code is somewhat written imperatively. 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.

 What are Object Oriented Abusers:

Inaccurate or undeveloped OO principles. 

Previous Code Smell articles

Bloaters	Object Oriented Abusers
Code Smells, Refactoring Comments– Bloaters: Day 1 Code Smells, Refactoring Long Functions– Bloaters: Day 2 Code Smells, Refactoring Long Parameter Lists– Bloaters: Day 3 Code Smells, Refactoring Data Clumps Including Parameter Lists– Bloaters: Day 4 Code Smells, Refactoring Long Script Includes– Bloaters: Day 5 Code Smells, Primitive Obsession– Bloaters: Day 6	Code Smells, Switch Statement – Object Oriented Abusers: Day 7

Day 8 - Refactoring Temporary Field

A Temporary field is a variable that isn't really needed, or used in a very limited situation:

Note: Temporary fields can have their uses cases.

Examples of Temporary fields

• is used in very specific cases
• instead of being passed to other methods, the method reaches out to set or read its value
• Used to track changing values as it passes through logic
• has no functional need other than being a placeholder

Solution

Extract temporary fields be extracted into their own SI, or replace them by passing them as parameters to methods. In a nutshell, structuring code logically often solves the problem. This is done by extracting the variables unrelated to each other into their own SI, and removing those without a real purpose.

Why Temporary Fields smells

• leads to clutter
• identify incomplete code structures
• can increase debugging complexity
• indicative of a mixture of concerns (not always bad, more like how it's done)

 Basic Examples of Temporary Fields

function nameToObject (name) {
   var fullName = name.split(' ');
   var firstName = fullName[0];
   var lastName = lastName[1];

   var name = {
      firstName: firstName,
      lastName: lastName
   };

   return name;
}

function sum (a, b) {
  var total = a + b;
  return total;

}

Example Solution

function nameToObject (name) {
   var fullName = name.split(' ');

   return {
      firstName: fullName[0],
      lastName: fullName[1]
   };

}

function sum (a, b) {
  return a + b;

}

In case of Objects and longer methods, temporary fields can lead to bloating, handling too much responsibility, even confusion and increased interpretation times...

Places where their use can help is when breaking down LISP like call points in functional application programming, or when calling curried methods, etc. Rather than a one liner, the line is broken down into multiple.

Predicate(aPredicateStrategy)(aFailureFunction,aSuccessFunction)(valueToCheck)

oneMethod(twoMethod(threeMethod(value)));

 Happy SNowing...

[Javier Arroyo]

Rather than adding all of this code to the original post. This comment will fully refactor a GlideRecord query into a one liner, just to display the transition of how Temporary Fields are not always necessary. I didn't really eliminate them all from the utility functions, but... it's a start. 

The Original Entry Point Function

function changesToObjects (fields) {

    var changes = new GlideRecord('change_request');
    changes.setLimit(10);
    changes.query();

    var objectified = [];

    while (changes.next()) {
        var change = {};

        change.number = changes.getValue('number');
        change.short_description = changes.getValue('short_description');

        objectified.push(change);

    }

    return objectified;
}

Call Site

gs.debug(JSON.stringify(changesToObjects(['number', 'short_description']), null, 2));

Output

 *** Script: [DEBUG] [
  {
    "number": "CHG0001046",
    "short_description": "DLP alert"
  },
  {
    "number": "CHG0001009",
    "short_description": "Admin Privileges Abused"
  },
  {
    "number": "CHG0001028",
    "short_description": "Person in accounting installed an Excel add-on"
  },
  {
    "number": "CHG0001004",
    "short_description": "Malware detected on server"
  },
  {
    "number": "CHG0001045",
    "short_description": "rogue wireless device detected"
  },
  {
    "number": "CHG0001035",
    "short_description": "Someone is remoted into my machine"
  },
  {
    "number": "CHG0000024",
    "short_description": "Clear BGP sessions on a Cisco router"
  },
  {
    "number": "CHG0001007",
    "short_description": "Physical breach of office"
  },
  {
    "number": "CHG0000003",
    "short_description": "Roll back Windows SP2 patch"
  },
  {
    "number": "CHG0001034",
    "short_description": "Unauthorized Network scans"
  }
]

First the refactored samples will be added, followed by functions from a made believe utility class, finally the result which should be the same as the above output.

Refactoring Sample 1

function changesToObjects (fields) {

    var changes = getSomeChanges();
    var getValuesFn = extractValuesFromFields(fields);

    var result = [];

    while (changes.next()) {
        result.push(getValuesFn(changes));

    }

    return result;
}

GlideRecord removed into its own function and function to extract values introduced, and temporary field change removed from within the while.

Refactoring Sample 2

function changesToObjects (fields) {

    var changes = getSomeChanges();
    var getValuesFn = extractValuesFromFields(fields);
    var convertedChanges = grWhile(getValuesFn, changes);

    return convertedChanges;

}

utility function introduced to while (GlideRecord.next(gr)) given a function to execute and gr.query result, then the result temporary variable removed.

Refactoring Sample 3

function changesToObjects (fields) {
    return grWhile(extractValuesFromFields(fields), getSomeChanges());

}

All temporary variables removed.

Utility functions (Think of these as having come from a pre-existing library that can be leveraged from other places)

function getSomeChanges () {
    var gr = new GlideRecord('change_request');
    gr.setLimit(10);
    gr.query();

    return gr;

}

function grWhile (morphism, grRecordSet) {
    var result = [];

    while (grRecordSet.next()) {       
        result.push(morphism(grRecordSet));

    }

    return result;
}

function getValue (change) {

    return function get (reducedValue, field) {        
        reducedValue[field] = change.getValue(field);

        return reducedValue;

    };
}

function extractValuesFromFields (fields) {

    return function fromGR (change) {
        return fields.reduce(getValue(change), {});

    };

}

Call Site

gs.debug(JSON.stringify(changesToObjects(['number', 'short_description']), null, 2));

Output

*** Script: [DEBUG] [
  {
    "number": "CHG0001046",
    "short_description": "DLP alert"
  },
  {
    "number": "CHG0001009",
    "short_description": "Admin Privileges Abused"
  },
  {
    "number": "CHG0001028",
    "short_description": "Person in accounting installed an Excel add-on"
  },
  {
    "number": "CHG0001004",
    "short_description": "Malware detected on server"
  },
  {
    "number": "CHG0001045",
    "short_description": "rogue wireless device detected"
  },
  {
    "number": "CHG0001035",
    "short_description": "Someone is remoted into my machine"
  },
  {
    "number": "CHG0000024",
    "short_description": "Clear BGP sessions on a Cisco router"
  },
  {
    "number": "CHG0001007",
    "short_description": "Physical breach of office"
  },
  {
    "number": "CHG0000003",
    "short_description": "Roll back Windows SP2 patch"
  },
  {
    "number": "CHG0001034",
    "short_description": "Unauthorized Network scans"
  }
]

Original Snippet Entry Point Function	Final Refactored Entry Point Function
function changesToObjects (fields) { var changes = new GlideRecord('change_request'); changes.setLimit(10); changes.query(); var objectified = []; while (changes.next()) { var change = {}; change.number = changes.getValue('number'); change.short_description = changes.getValue('short_description'); chanobjectifiedges.push(change); } return objectified; }	function changesToObjects (fields) { return grWhile(extractValuesFromFields(fields), getSomeChanges()); }