Refactor this method to reduce its Cognitive Complexity from 17 to the 15 allowed

解决代码嵌套严重提示的方法:
关于复杂度的计算可以参考 Cognitive Complexity 认知复杂度

简单来讲就是以下几点:

  1. if/else/for/while 各记一点, if语句相当于一个if
  2. 嵌套一层加1点
  3. try/catch/finally 只有catch记一点,其它不记, catch嵌套在其它里面不记嵌套, 其它嵌套在catch里面需要记嵌套.
  4. 连续的||记一点,连续的&&记一点
  5. break/continue 记一点,不考虑嵌套

例如:

    if(a || b || c) { //+2, if +1, 连续|| +1
    }
    if (a || b && d || e) { //+4, if +1, 剩余的||和&&不连续, 所以各 +1
    }

    if(a) {  //+1
        try {
            while (b) { //+2, 其中嵌套1层
                if (c || f) {  //+4, 其中嵌套2层, || +1, ||不考虑嵌套
                    int s =1;
                    break; //+1
                }
            }
        } catch (e) { // +1 不考虑嵌套
            if(h) { //+3 嵌套2层(if和catch)
            }
        }
    }
    //以上总共复杂度为18

以下转自: https://stackoverflow.com/questions/62872718/how-to-refactor-this-function-to-reduce-its-cognitive-complexity-from-17-to-the

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 complexityis 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 14instead 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.

最后编辑于
©著作权归作者所有,转载或内容合作请联系作者
  • 序言:七十年代末,一起剥皮案震惊了整个滨河市,随后出现的几起案子,更是在滨河造成了极大的恐慌,老刑警刘岩,带你破解...
    沈念sama阅读 215,463评论 6 497
  • 序言:滨河连续发生了三起死亡事件,死亡现场离奇诡异,居然都是意外死亡,警方通过查阅死者的电脑和手机,发现死者居然都...
    沈念sama阅读 91,868评论 3 391
  • 文/潘晓璐 我一进店门,熙熙楼的掌柜王于贵愁眉苦脸地迎上来,“玉大人,你说我怎么就摊上这事。” “怎么了?”我有些...
    开封第一讲书人阅读 161,213评论 0 351
  • 文/不坏的土叔 我叫张陵,是天一观的道长。 经常有香客问我,道长,这世上最难降的妖魔是什么? 我笑而不...
    开封第一讲书人阅读 57,666评论 1 290
  • 正文 为了忘掉前任,我火速办了婚礼,结果婚礼上,老公的妹妹穿的比我还像新娘。我一直安慰自己,他们只是感情好,可当我...
    茶点故事阅读 66,759评论 6 388
  • 文/花漫 我一把揭开白布。 她就那样静静地躺着,像睡着了一般。 火红的嫁衣衬着肌肤如雪。 梳的纹丝不乱的头发上,一...
    开封第一讲书人阅读 50,725评论 1 294
  • 那天,我揣着相机与录音,去河边找鬼。 笑死,一个胖子当着我的面吹牛,可吹牛的内容都是我干的。 我是一名探鬼主播,决...
    沈念sama阅读 39,716评论 3 415
  • 文/苍兰香墨 我猛地睁开眼,长吁一口气:“原来是场噩梦啊……” “哼!你这毒妇竟也来了?” 一声冷哼从身侧响起,我...
    开封第一讲书人阅读 38,484评论 0 270
  • 序言:老挝万荣一对情侣失踪,失踪者是张志新(化名)和其女友刘颖,没想到半个月后,有当地人在树林里发现了一具尸体,经...
    沈念sama阅读 44,928评论 1 307
  • 正文 独居荒郊野岭守林人离奇死亡,尸身上长有42处带血的脓包…… 初始之章·张勋 以下内容为张勋视角 年9月15日...
    茶点故事阅读 37,233评论 2 331
  • 正文 我和宋清朗相恋三年,在试婚纱的时候发现自己被绿了。 大学时的朋友给我发了我未婚夫和他白月光在一起吃饭的照片。...
    茶点故事阅读 39,393评论 1 345
  • 序言:一个原本活蹦乱跳的男人离奇死亡,死状恐怖,灵堂内的尸体忽然破棺而出,到底是诈尸还是另有隐情,我是刑警宁泽,带...
    沈念sama阅读 35,073评论 5 340
  • 正文 年R本政府宣布,位于F岛的核电站,受9级特大地震影响,放射性物质发生泄漏。R本人自食恶果不足惜,却给世界环境...
    茶点故事阅读 40,718评论 3 324
  • 文/蒙蒙 一、第九天 我趴在偏房一处隐蔽的房顶上张望。 院中可真热闹,春花似锦、人声如沸。这庄子的主人今日做“春日...
    开封第一讲书人阅读 31,308评论 0 21
  • 文/苍兰香墨 我抬头看了看天上的太阳。三九已至,却和暖如春,着一层夹袄步出监牢的瞬间,已是汗流浃背。 一阵脚步声响...
    开封第一讲书人阅读 32,538评论 1 268
  • 我被黑心中介骗来泰国打工, 没想到刚下飞机就差点儿被人妖公主榨干…… 1. 我叫王不留,地道东北人。 一个月前我还...
    沈念sama阅读 47,338评论 2 368
  • 正文 我出身青楼,却偏偏与公主长得像,于是被迫代替她去往敌国和亲。 传闻我的和亲对象是个残疾皇子,可洞房花烛夜当晚...
    茶点故事阅读 44,260评论 2 352

推荐阅读更多精彩内容