Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CognitiveComplexity: count else/else-if as one complexity #5458

Merged
merged 3 commits into from Oct 24, 2022

Conversation

t-kameyama
Copy link
Contributor

Fixes #5447

} else if (condition) { // +2
while(true) { // +3
}
} else { // +2
while(true) { // +3
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From #5447 and paper of Cognitive Complexity,

No nesting increment is assessed for these structures because the mental cost has already been paid when reading the if.

else and else if seem to be counted as +1 complexity only (do not include nesting level increments).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@sanggggg sanggggg Oct 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry @t-kameyama, I said something that could be misunderstood.
What I'm trying to say is

else if and else

  • should add +1 complexity (Increments)
  • should increase nesting level (Nesting Level)
  • should not add complexity by nesting level (no Nesting increments)

Screen Shot 2022-10-23 at 1 02 10 AM


Thus, the complexity of the test code should be 39

                fun test(condition: Boolean) {
                    if (condition) { // +1
                        if (condition) { // +2
                            while(true) { // +3
                            }
                        } else if (condition) { // +1
                            while(true) { // +3
                            }
                        } else if (condition) { // +1
                            while(true) { // +3
                            }
                        } else { // +1
                            while(true) { // +3
                            }
                        }
                    } else if (condition) { // +1
                        if (condition) { // +2
                            while(true) { // +3
                            }
                        } else if (condition) // +1
                            while(true) { // +3
                            }
                    } else { // + 1
                        if (condition) { // +2
                            while(true) { // +3
                            }
                        } else // +1
                            while(true) { // +3
                            }
                    }
                    if (condition) { // +1
                    }
                }
                """
                // total 39 comple

The complexity suddenly increased, but I think this is a more real complexity (from the perspective of Cognitive Complexity)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

@BraisGabin BraisGabin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. @sanggggg it does follow what you were asking, right?

Copy link
Contributor

@sanggggg sanggggg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, thank you for implementation! 🚀

if (element.expression is KtIfExpression) {
super.visitKtElement(element)
} else {
complexity++
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯 since an else branch only increment the complexity without adding nesting

@chao2zhang chao2zhang merged commit 3f5489c into detekt:main Oct 24, 2022
21 checks passed
@t-kameyama t-kameyama deleted the issue_5447 branch October 24, 2022 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CognitiveComplexity should count else if and else as +1 complexity
4 participants