Refactorisez cette méthode pour réduire sa complexité cognitive de 21 aux 15 autorisés. Comment refactoriser et réduire la complexité

Tous ceux-là || additionnez simplement et cela ressemble à une mauvaise pratique. Vous pouvez déplacer this.deviceDetails = {... vers sa propre fonction de mappage pour une solution rapide.

si vous utilisez typescript 3.7 ou version ultérieure, vous pouvez utiliser le chaînage facultatif pour simplement certaines de vos conditions.

• device.deviceManager && device.deviceManager.managerId || null

deviendrait

• device.deviceManager? .managerId || null

Un peu d'informations sur le fonctionnement de la complexité cyclomatique et pourquoi vous devriez la maintenir à un niveau bas

Tout d'abord, il est important de comprendre comment " Complexité cognitive " fonctionne comme par rapport à " Complexité cyclomatique ". La complexité cognitive prend en compte la complexité perçue par le cerveau humain. C'est pourquoi il n'indique pas simplement le nombre de chemins conditionnels (simplifié le nombre de conditionnelles plus 1 pour l'instruction de retour).

La complexité cyclomatique par contre aussi considère les conditions imbriquées (par exemple, un if à l'intérieur d'une instruction if), ce qui rend encore plus difficile la lecture et la compréhension du code du point de vue d'un humain.

L'exemple suivant de la documentation SonarQube ( https://www.sonarsource.com/docs/CognitiveComplexity.pdf ) ce que j'essaie d'expliquer:

this.deviceDetails = this.data && { ...this.data.deviceInfo } || {}; // +2
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), 
        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 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) {
    return infoItem || ''; // +1 for the binary operator
}

Sachez donc que les opérations binaires ajoutent à la complexité mais que les conditions imbriquées ajoutent un score de plus 1 pour chaque condition imbriquée. Ici, la complexité cognitive serait de 6, tandis que la complexité cyclomatique ne serait que de 4 (un pour chaque conditionnel et un pour le chemin de retour);

Si vous rendez votre code plus lisible pour un humain, par exemple en extrayant des méthodes à partir de lignes contenant des conditions, vous obtenez à la fois une meilleure lisibilité et moins de complexité cyclomatique.

Bien que le code que vous avez fourni n'ait pas de conditionnelles imbriquées, je pense qu'il est important de comprendre d'abord comment fonctionne le calcul de complexité cyclomatique et pourquoi c'est une bonne idée de le garder bas.

[TL; DR] Une approche possible pour refactoriser votre code dans une version moins complexe et mieux lisible

Voyons d'abord comment le calcul de la complexité de votre code est effectué comme indiqué par les commentaires:

if (this.data && this.data.deviceInfo) { // +1 for the if conditaionl, +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
}

Cela pourrait être une version refactorisée de votre code qui (à partir d'un manuel rapide sans véritable analyse SonarQube) devrait réduire la complexité cognitive à 12. (Veuillez noter qu'il ne s'agit que d'un calcul manuel.)

Cela peut être fait en appliquant des refactorisations simples telles que la méthode d'extraction et / ou move (voir aussi Martin Fowler, https://refactoring.com/catalog/extractFunction.html ).

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

Avec la méthode d'extraction simple refactorise beaucoup de duplications (voir la fonction getInfoItem ()) a également été éliminé , ce qui permet de réduire facilement la complexité et d'augmenter la lisibilité .

Pour être honnête, j'irais même plus loin et restructurerais encore plus votre code afin que la logique de vérification des éléments vides et de définition d'une valeur par défaut (ici une chaîne vide) lors de la fourniture des détails de l'appareil soit effectuée par la classe de l'appareil ou les détails d'un appareil classe elle-même pour avoir une meilleure cohésion des données et de la logique qui opère sur ces données. Mais comme je ne connais pas le reste du code, cette refactorisation initiale devrait vous amener un peu plus loin vers une meilleure lisibilité et moins de complexité.