Refactor this method to reduce its Cognitive Complexity from 21 to the 15 allowed. How to refactor and reduce the complexity

Question

how to reduce the complexity of the given piece of code? I am getting this error in Sonarqube---> Refactor this method to reduce its Cognitive Complexity from 21 to the 15 allowed.

this.deviceDetails = this.data && {...this.data.deviceInfo} || {};
    if (this.data && this.data.deviceInfo) {
      this.getSessionInfo();
      // tslint:disable-next-line: no-shadowed-variable
      const { device, driver, ipAddress, port, active, connectionType } = this.data.deviceInfo;
      this.deviceDetails = {
        name: device.name || '',
        manufacturer: device.manufacturer || '',
        deviceType: device.deviceType || '',
        model: device.model || '',
        description: device.description || '',
        managerId: device.deviceManager && device.deviceManager.managerId || null,
        locationId: device.location && device.location.locationId || null,
        active: device.active,
        connectionType: connectionType || null,
        driver_id: driver && driver.driverId || null,
        ipAddress: ipAddress || '',
        port: String(port) || '',
        connectionStatus: active,
      };
      this.oldDeviceDetails = {...this.deviceDetails};
      this.deviceLocation = device.location && device.location.locationId || null;
    } else {

Answer 1

A little information on how cognitive complexity works and why you should keep it low

First of all it is important to understand how "Cognitive Complexity" works as compared to "Cyclomatic Complexity". Cognitive complexity takes into account the complexity perceived by the human brain. This is why it does not simply indicate the number of conditional paths (simplified the number of conditionals plus 1 for the return statement).

The cognitive complexity metric also considers nested conditions (e.g. an if inside an if, inside an if statement) which makes it even harder to read and understand the code from a human's perspective.

The following sample from the SonarQube documentation (https://www.sonarsource.com/docs/CognitiveComplexity.pdf) outlines what I'm trying to explain:

if (someVariableX > 3) { // +1
    if (someVariableY < 3) { // +2, nesting = 1
        if (someVariableZ === 8) { // +3, nesting = 2
            someResult = someVariableX + someVariableY - someVariableZ;
        }
    }
}

So be aware that binary operations add to the complexity but nested conditions even add a score of plus 1 for each nested condition. Here the cognitive complexity would be 6, while the cyclomatic complexity would only be 4 (one for each conditional and one for the return path);

If you make your code more readable for a human, e.g. by extracting methods from lines that contain conditionals you achieve both, better readability and less complexity.

Although the code you provided does not have nested conditionals I think it is important to first understand how cyclomatic and cognitive complexity calculation works and why it is a good idea to keep it low.

[TL;DR] A possible approach to refactor your code into a less complex and better readable version

Let's first look how the complexity calcuation is done for your code as outlined by the comments:

this.deviceDetails = this.data && {  ...this.data.deviceInfo } || {}; // +2
if (this.data && this.data.deviceInfo) { // +1 for the if condition, +1 for the binary operator
    this.getSessionInfo();

    const { device, driver, ipAddress, port, active, connectionType } =             
    this.data.deviceInfo;
    this.deviceDetails = {
        name: device.name || '', // +1 for the binary operator
        manufacturer: device.manufacturer || '', // +1 for the binary operator
        deviceType: device.deviceType || '', // +1 for the binary operator
        model: device.model || '', // +1 for the binary operator
        description: device.description || '', // +1 for the binary operator
        managerId: device.deviceManager && device.deviceManager.managerId || null, // +2 for the varying binary operators
        locationId: device.location && device.location.locationId || null, // +2 for the varying binary operator
        active: device.active,
        connectionType: connectionType || null, // +1 for the binary operator
        driver_id: driver && driver.driverId || null, // +2 for the varying binary operator
        ipAddress: ipAddress || '', // +1 for the binary operator
        port: String(port) || '', // +1 for the binary operator
        connectionStatus: active,
    };
    this.oldDeviceDetails = { ...this.deviceDetails };
    this.deviceLocation = device.location && device.location.locationId || null; // +2 for the varying binary operator
} else { // +1 for the else path 
    // some statement
}

So assuming my math is correct, this sums up to the cognitive complexity of 21 as reported by SonarQube.

The following code sample shows how your code can be refactored to a version which should lower the cognitive complexity down to 12. (Please be aware that this is just a manual calculation.)

It can be done by applying simple refactorings such as extract method and/or move method (see also Martin Fowler, https://refactoring.com/catalog/extractFunction.html).

this.deviceDetails = getDeviceInfo();
if (deviceInfoAvailable()) { // +1 for the if statement
    this.getSessionInfo();
    // tslint:disable-next-line: no-shadowed-variable
    const { device, driver, ipAddress, port, active, connectionType } = this.data.deviceInfo;
    this.deviceDetails = {
        name: getInfoItem(device.name), 
        manufacturer: getInfoItem(device.manufacturer),
        deviceType: getInfoItem(device.deviceType),
        model: getInfoItem(device.model),
        description: getInfoItem(device.description), 
        managerId: getManagerId(device),
        locationId: getDeviceLocation(device),
        active: device.active,
        connectionType: getInfoItem(connectionType, null), 
        driver_id: getDriverId(driver), 
        ipAddress: getInfoItem(ipAddress), 
        port: getInfoItem(port), 
        connectionStatus: active,
    };
    this.oldDeviceDetails = { ...this.deviceDetails };
    this.deviceLocation = getDeviceLocation(device);
} else { // +1 for the else
    // some statement
}

function getDeviceInfo()
{
    return this.data && { ...this.data.deviceInfo } || {}; // +2
}

function getDriverId(driver) {
    return driver && driver.driverId || null; // +2 for the binary operators
}

function getDeviceLocation(device) {
    return device.location && device.location.locationId || null; // +2 for the binary operators
}

function getManagerId(device) {
    return device.deviceManager && device.deviceManager.managerId || null; // +2 for the binary operators
}

function deviceInfoAvailable() {
    return this.data && this.data.deviceInfo; // +1 for the binary operator
}

function getInfoItem(infoItem, defaultValue = '') {
    return infoItem || defaultValue; // +1 for the binary operator
}

With the simple extract method refactorings lots of duplications (see getInfoItem() function) got eliminated as well which makes it easy to reduce the complexity and increase the readability.

To be honest, I would even go some steps further and restructure your code even more so that the logic for checking for empty items and setting a default value (here an empty string) when providing the device details is done by the device class or a device details class itself to have better cohesion of the data and the logic that operates on that data. But as I don't know the rest of the code this inital refactoring should get you one step further to better readability and less complexity.

Note: We could even go one step further and perform a so called Replace Nested Conditional with Guard Clauses refactoring (sometimes also referred to as "early return" or "invert if statement).

This could result in the code shown below and further reduce the cognitive complexity by one due to the elimination of the else statement, resulting in a final cognitive complexity of 11. The extracted functions are the same and therefore not listed again here...

this.deviceDetails = getDeviceInfo();
if (deviceInfoAvailable()) { // +1 for the if statement
    // some statement
    return; // return the same way as in the eliminated else clause
}

this.getSessionInfo();
const { device, driver, ipAddress, port, active, connectionType } = this.data.deviceInfo;
this.deviceDetails = {
  name: getInfoItem(device.name), 
  manufacturer: getInfoItem(device.manufacturer),
  deviceType: getInfoItem(device.deviceType),
  model: getInfoItem(device.model),
  description: getInfoItem(device.description), 
  managerId: getManagerId(device),
  locationId: getDeviceLocation(device),
  active: device.active,
  connectionType: getInfoItem(connectionType, null), 
  driver_id: getDriverId(driver), 
  ipAddress: getInfoItem(ipAddress), 
  port: getInfoItem(port), 
  connectionStatus: active,
};
this.oldDeviceDetails = { ...this.deviceDetails };
this.deviceLocation = getDeviceLocation(device);