-
Notifications
You must be signed in to change notification settings - Fork 104
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
SONARPHP-683 Rule: Cognitive Complexity of functions should not be too high #173
Conversation
| public void visitCatchBlock(CatchBlockTree tree) { | ||
| complexity.addComplexityWithNesting(tree.catchToken()); | ||
|
|
||
| complexity.incNesting(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems a perfect case for FP, so I propose to remove all this duplication with a little wrapper in CognitiveComplexity :
public void nestAndDo(Runnable action) { level++; action.run(); level--;}
Which will let you replace all these things with calls like complexity.nestAndDo(() -> super.visitCatchBlock(tree));
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| } | ||
|
|
||
| @Override | ||
| public void visitBinaryExpression(BinaryExpressionTree tree) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to first flatten the BinaryExpressionTree in a list of operators that reproduces the original syntax : [AND,OR,AND] and only after we have that list we calculate the complexity on it by scanning from left to right.
The algorithm to flatten could be something like :
list = new_empty_list
i = 0
flatten(i, list, tree)
function flatten(i, list, tree)
list.add(i,tree)
flatten(i, list, tree.left)
flatten(++i, list, tree.right)
end_function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
44e13d8 to
81affa1
Compare
| statements.forEach(statementTree -> statementTree.accept(this))); | ||
| } | ||
|
|
||
| private void visitWithNesting(Runnable action) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
b1b9ee8 to
94056f0
Compare


No description provided.