'How can I reduce cyclomatic complexity for the code to be acceptable by sonar
This function is not working as expected in the sonar
Solution 1:[1]
A simplified way of thinking of Sonar's cyclomatic complexity metric would be "how many logical branches and loops do I have to reason about to understand this function?" There are a few approaches you can take:
- Split your function up. If the function is doing multiple things, try splitting each thing out into its own function
- Reduce branches in your code. Often times, an if/else statement can be a conditional assignment instead.
Cyclomatic complexity doesn't even cover some other things hard to follow in your code:
- You are often duplicating segments code instead of storing things off in temporary variables. Duplicating code not only makes someone reading the code have more work to do, but it also means you're forcing the program to do this extra work.
- Because you're using
var
throughout, someone reading your code has to look through your entire function to know whether or not a variable will be reassigned at different points. Usingconst
when something won't change lets us know whether we don't have to worry about this any longer. - Some of your if statement conditions are complex. Try to refactor them into more readable temporary variables.
Here's my stab at reducing your cyclomatic complexity and code readability:
function validateSignInForm(signinErrorMessage) {
let bFinalSubmit = true;
if (!validateFields()) {
bFinalSubmit = false;
}
const errMainError = (typeof signinErrorMessage === 'undefined')
? configuration.messages.signinIncomplete;
: signinErrorMessage;
if (errMainError === configuration.messages.notFound) {
trp.fai.common.log(errMainError);
bFinalSubmit = false;
}
const errMsg = $('#signin-mainerr-message');
const errContainer = $('#signin-ui-notification-container-mainerr');
if (!bFinalSubmit) {
errMsg.html(errMainError);
errContainer.show();
} else {
errMsg.html('');
errContainer.hide();
}
return bFinalSubmit;
}
function validateFields() {
let canSubmit = true;
configuration.required.signin.forEach(field => {
const fieldName = field.name;
const fieldValue = $.trim(field.value);
const fieldElement = $('#' + fieldName);
const fieldLabel = fieldElement
.closest('li')
.find('label')
if (
(fieldValue === '' || (fieldName === 'username' && fieldValue === 'Your email address'))
&& $('#' + field.id).is(':visible')
) {
fieldElement.addClass('ui-red-border');
fieldLabel.addClass('ui-red-text');
canSubmit = false;
} else {
fieldElement.removeClass('ui-red-border');
fieldLabel.removeClass('ui-red-text');
$('#' + fieldName + 'Field').removeClass('registerFieldError');
if (configuration.required.signin[i].onchange) {
configuration.required.signin[i].onchange();
}
}
});
return canSubmit;
}
Solution 2:[2]
A quick fix will be to extract methods into smaller method and avoid if else with switch statement or by polymorphisms.
Sources
This article follows the attribution requirements of Stack Overflow and is licensed under CC BY-SA 3.0.
Source: Stack Overflow
Solution | Source |
---|---|
Solution 1 | Jacob |
Solution 2 | Kumar Shanoo |