1. ホーム
  2. angular

[解決済み] このメソッドをリファクタリングして、認知的複雑度を21から許容される15に下げます。リファクタリングして複雑さを軽減する方法

2022-02-19 01:31:05

質問内容

与えられたコードの複雑さを軽減する方法は?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 {

解決方法は?

認知的複雑性の仕組みと、それを低く抑えるべき理由について、少しご紹介します。

まず、"quot "がどのようなものかを理解することが重要です。 認知的複雑性 と比べて、「"」はどのように機能するのでしょうか? 循環的複雑性 "です。認知的複雑性は、人間の脳が認識する複雑さを考慮したものです。そのため、単純に条件パスの数を示すのではない(return文の場合は条件パスの数+1を簡略化する)。

その 認知的複雑性 メトリックはまた ネストされた条件を考慮する (例:ifの中にifがある、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になりますが、循環的複雑度は4(各条件と戻り経路に1)で済みます。

例えば、条件分岐を含む行からメソッドを抽出するなどして、人間にとって読みやすいコードにすれば、読みやすさと複雑さの両方を達成することができるのです。

提供されたコードにはネストした条件文はありませんが、まず、サイクロマティック計算と認知的複雑さの計算の仕組みと、なぜそれを低く抑えることが良いのかを理解することが重要だと思います。

[TL;DR] コードをより複雑でなく、より読みやすいものにリファクタリングするために可能なアプローチ

まず、コメントにあるように、あなたのコードに対して複雑さの計算がどのように行われるかを見てみましょう。

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
}

私の計算が正しいと仮定すると、SonarQubeが報告した認知的複雑さは21ということになりますね。

次のコードサンプルは、あなたのコードがどのようにリファクタリングされ、どのようなバージョンになるかを示しています。 認知的複雑性を12に下げる . (あくまで手計算であることをご了承ください)。

を適用することで可能です。 シンプルリファクタリング というような メソッド抽出 やmoveメソッド(Martin Fowlerも参照。 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
}

シンプルなextractメソッドのリファクタリングで 重複が多い (getInfoItem()関数を参照) 削除された と同様に は、複雑さを軽減し、読みやすさを向上させることが容易になります .

正直なところ、私はさらに一歩進んで、デバイスの詳細を提供する際に空の項目をチェックしデフォルト値(ここでは空文字列)を設定するロジックを、デバイスクラスまたはデバイス詳細クラス自身が行うようにコードを再構築し、データとそのデータで動作するロジックの一貫性をより高めることも考えています。しかし、私は残りのコードを知らないので、この最初のリファクタリングで、より読みやすく、より複雑でなくなるように一歩前進することができるはずです。