Выполните рефакторинг этого метода, чтобы снизить его когнитивную сложность с 21 до 15 разрешенных. Как провести рефакторинг и уменьшить сложность - PullRequest
0 голосов
/ 09 июля 2020

как уменьшить сложность данного фрагмента кода? Я получаю эту ошибку в Sonarqube ---> Выполните рефакторинг этого метода, чтобы снизить его когнитивную сложность с 21 до 15 разрешенных.

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 {

Ответы [ 3 ]

0 голосов
/ 09 июля 2020

если вы используете машинописный текст 3.7 или новее, вы можете использовать необязательную цепочку просто для некоторых из ваших условий.

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

превратится в

  • device.deviceManager? .managerId || ноль
0 голосов
/ 13 июля 2020

Небольшая информация о том, как работает cyclomati c сложность и почему вы должны держать ее на низком уровне

Прежде всего, важно понять, как « Cognitive Complexity"работает по сравнению с" Cyclomati c Complexity". Когнитивная сложность учитывает сложность, воспринимаемую человеческим мозгом. Вот почему он не просто указывает количество условных путей (упрощенное количество условных операторов плюс 1 для оператора возврата).

цикломати c сложность , с другой стороны, также учитывает вложенные условия (например, if внутри оператора if), что еще больше затрудняет чтение и понимание кода с точки зрения человека.

Следующий пример из документации SonarQube ( https://www.sonarsource.com/docs/CognitiveComplexity.pdf) обрисовывает в общих чертах то, что я пытаюсь объяснить:

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

Так что имейте в виду, что бинарные операции увеличивают сложность, но вложенные условия добавляют оценку плюс 1 для каждого вложенного состояние. Здесь когнитивная сложность будет равна 6, а сложность cyclomati c будет только 4 (по одному для каждого условного и по одному для пути возврата);

Если вы сделаете свой код более читабельным для человека, например извлекая методы из строк, содержащих условные выражения, вы добиваетесь как лучшей читаемости, так и меньшей цикломати c сложности.

Хотя в предоставленном вами коде нет вложенных условных операторов, я думаю, что важно сначала понять, как cyclomati c вычисление сложности работает, и почему рекомендуется поддерживать его на низком уровне.

[TL; DR] Возможный подход к рефакторингу вашего кода в менее сложную и более читаемую версию

Давайте сначала посмотрим, как выполняется расчет сложности для вашего кода, как указано в комментариях:

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
}

Это может быть отредактированная версия вашего кода, которая (из быстрого ручного подсчета без реального SonarQube анализ) должен снизить когнитивную сложность до 12. (Помните, что это s - это просто ручное вычисление.)

Это можно сделать, применив простые рефакторинги, такие как метод извлечения и / или метод перемещения (см. также Мартин Фаулер, https://refactoring.com/catalog/extractFunction.html).

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
}

С помощью рефакторинга простого метода извлечения много дубликатов (см. Функцию getInfoItem ()) также было удалено , что упрощает упрощение и увеличить читаемость .

Если честно, я бы даже go сделал несколько шагов дальше и реструктурировал ваш код еще больше, чтобы logi c для проверки пустых элементов и установки значения по умолчанию (здесь пустая строка), когда предоставление сведений об устройстве выполняется классом устройства или самим классом сведений об устройстве, чтобы обеспечить лучшую согласованность данных и logi c, который работает с этими данными. Но, поскольку я не знаю остальной части кода, этот начальный рефакторинг должен сделать вас еще на один шаг вперед к лучшей читаемости и меньшей сложности.

0 голосов
/ 09 июля 2020

Все эти || просто сложите, и это похоже на плохую практику. Вы можете перенести this.deviceDetails = {... на собственную функцию сопоставления для быстрого решения.

...