How to refactor this function to reduce its Cognitive Complexity from 17 to the 15 allowed

Question

• How to refactor this function to reduce the complexicity
• When I am using switch case at that time code complixity is more so how to reduce it

  How to achive this

var has = Object.prototype.hasOwnProperty



var toString = Object.prototype.toString


function isEmpty(val) {
 
  if (val == null) return true
  if ('boolean' == typeof val) return false
  if ('number' == typeof val) return val === 0
  if ('string' == typeof val) return val.length === 0
  if ('function' == typeof val) return val.length === 0
  if (Array.isArray(val)) return val.length === 0
  if (val instanceof Error) return val.message === ''
  if (val.toString == toString) {
    switch (val.toString()) {
      case '[object File]':
      case '[object Map]':
      case '[object Set]': {
        return val.size === 0
      }
      case '[object Object]': {
        for (var key in val) {
          if (has.call(val, key)) return false
        }

        return true
      }
    }
  }
  return false
}
module.exports = isEmpty

Answer 1

I recently gave an answer to a very similar question going a little more into detail about how cognitive complexity works (see https://stackoverflow.com/a/62867219/7730554).

But in general, I think it is important to understand that cognitive complexity is increased even more if there are nested conditions. This calculation is done this way because the human brain can deal a lot better with statements written in sequence rather than with nested conditions. So for each conditional statement (if, switch, for loop, etc.) there will be a +1 added to the complexity value. But for each nested condition another +1 is added on top of the last level. That means, an if inside an if will not only add +1, but +2. An if, inside an if, inside an if will than result in +1 for the first if, +2 for the second and +3 for the third if condition. If you want to dig deeper into this I recommend taking a look at: https://www.sonarsource.com/docs/CognitiveComplexity.pdf

So let's analyze where the high complexity values from your method originate first:

function isEmpty(val) {
    if (val == null) return true // +1 
    if ('boolean' == typeof val) return false // +1
    if ('number' == typeof val) return val === 0 // +1
    if ('string' == typeof val) return val.length === 0 // +1
    if ('function' == typeof val) return val.length === 0 // +1
    if (Array.isArray(val)) return val.length === 0 // +1
    if (val instanceof Error) return val.message === '' // +1
    if (val.toString == toString) { // +1
        switch (val.toString()) { // +2
            case '[object File]':
            case '[object Map]':
            case '[object Set]': {
                return val.size === 0
            }
            case '[object Object]': {
                for (var key in val) { // +3
                    if (has.call(val, key)) return false // +4
                }

                return true
            }
        }
    }
    return false
}

If you look at the comments I added you can easily see where the most problematic code concerning cyclomatic complexity is located. This also relates to the human readabilty of the code.

So one simply step to increase readability and at the same time reduce congnitive complexity is to look for options of "early returns".

To illustrate this, I simply inverted the statement *if (val.toString == toString)" to immediately return false if *val.toString != toString":

function isEmpty(val) {
    if (val == null) return true // +1 
    if ('boolean' == typeof val) return false // +1
    if ('number' == typeof val) return val === 0 // +1
    if ('string' == typeof val) return val.length === 0 // +1
    if ('function' == typeof val) return val.length === 0 // +1
    if (Array.isArray(val)) return val.length === 0 // +1
    if (val instanceof Error) return val.message === '' // +1
    if (val.toString != toString) { // +1
        return false;
    }
    
    switch (val.toString()) { // +1
        case '[object File]':
        case '[object Map]':
        case '[object Set]': {
            return val.size === 0
        }
        case '[object Object]': {
            for (var key in val) { // +2
                if (has.call(val, key)) return false // +3
            }
            return true
        }
    }
}  

Now the last switch statement can be executed outside the if statement and we reduced the nesting level by one. With that simple change the cognitive complexity has now dropped to 14 instead of 17.

You could then even go a step further and change the last case statement by extracting the return value into a variable and either extract a separate method out of the code block. This would reduce the complexity of the isEmpty() method even more.

And besides from extracting a method you can also use a declarative approach and use, for instance the Array method find() which would reduce the cognitive complexity even more.

To illustrate the idea I did both:

function isEmpty(val) {
    if (val == null) return true // +1 
    if ('boolean' == typeof val) return false // +1
    if ('number' == typeof val) return val === 0 // +1
    if ('string' == typeof val) return val.length === 0 // +1
    if ('function' == typeof val) return val.length === 0 // +1
    if (Array.isArray(val)) return val.length === 0 // +1
    if (val instanceof Error) return val.message === '' // +1
    if (val.toString != toString) { // +1
        return false;
    }
    
    return checkForComplexTypes(val)
}

function checkForComplexTypes(val) {
    var result = null
    switch (val.toString()) { // +1
        case '[object File]':
        case '[object Map]':
        case '[object Set]': {
            result = val.size === 0
        }
        case '[object Object]': {
            result = Object.keys(val).find(key => has.call(val, key))
        }
        return result
    }
}

This should bring down the cognitive complexity of the isEmpty() method to 8 and the whole code including the extracted checkForComplexTypes() function to a complexity score of 9.

Note: JavaScript is not my major language at the moment so I cannot fully guarantee the correctness of the last refactoring step.

Answer 2

If you cannot split the function or use OOP approach, you can use array of functions and iterate over it:

const has = Object.prototype.hasOwnProperty;
const toString = Object.prototype.toString;

function isEmpty(val) {
    let isEmpty = null;

    const checkFunctions = [
        (val) => 'boolean' === typeof val ? false : null,
        (val) => 'number' === typeof val ? val === 0 : null,
        (val) => ['string', 'function'].includes(typeof val) ? val.length === 0 : null,
        (val) => Array.isArray(val) ? val.length === 0 : null,

        (val) => val instanceof Error ? val.message === '' : null,

        (val) => val.toString == toString && ['[object File]', '[object Map]', '[object Set]'].includes(val.toString()) ? val.size === 0 : null,
        (val) => {
            if (val.toString == toString && val.toString() === '[object Object]') {
                for (var key in val) {
                    if (has.call(val, key)) return false
                }
                return true;
            }
        }
    ];

    for (let i = 0; i < checkFunctions.length; i++) {
        isEmpty = checkFunctions[i](val);
        if (isEmpty !== null) {
            return isEmpty;
        };
    }
}

console.log(isEmpty(''), true);
console.log(isEmpty('Hallo'), false);
console.log(isEmpty(0), true);
console.log(isEmpty(1), false);
console.log(isEmpty({}), true);
console.log(isEmpty({a: 1}), false);

You can also extend core types of JS and then instead of isEmpty(val) write val.isEmpty(). For example:

String.prototype.isEmpty = function() {return this.length === 0}
Array.prototype.isEmpty = function() {return this.length === 0}

console.log("".isEmpty(), true);
console.log("foo".isEmpty(), false);
console.log([].isEmpty(), true);
console.log([1,2,3].isEmpty(), false);