About Cyclomatic Complexity

I read one of the blog posts in dev.to about this Cyclomatic Complexity and I couldn't resist also not talking about this one. Because I see a lot of programmers making this mistake without knowing it.

So just today I checked my email and saw there is an automatic email containing changeset from one of my co-workers check-in this code:

function CheckEventBooking(EventMaintenanceInfo) {
    var AllowOverbooking, targetedbalance, Waitinglist, OverBookingpax;
    if (EventMaintenanceInfo.results[0].gent_AllowOverBooking != null) {
        AllowOverbooking = EventMaintenanceInfo.results[0].gent_AllowOverBooking;
    }
    if (EventMaintenanceInfo.results[0].gent_TargetedPAXBalance != null) {
        targetedbalance = EventMaintenanceInfo.results[0].gent_TargetedPAXBalance;
    }
    if (EventMaintenanceInfo.results[0].gent_IsWaitListAllowed != null) {
        Waitinglist = EventMaintenanceInfo.results[0].gent_IsWaitListAllowed;
    }
    if (EventMaintenanceInfo.results[0].gent_OverbookingPax != null) {
        OverBookingpax = EventMaintenanceInfo.results[0].gent_OverbookingPax;
    }
    //checking the balance
    if (targetedbalance == 0) {
        if (AllowOverbooking || Waitinglist) {
            if (AllowOverbooking && Waitinglist && OverBookingpax > 0) {
                var confirm = window.confirm("OverBooking & Waitinglist is allowed do you want to Confirm or Waitlist");
                if (confirm) {
                    Xrm.Page.getAttribute("gent_confirmation").setValue("Confirm");
                }
                else {
                    Xrm.Page.getAttribute("gent_confirmation").setValue("Waitinglist");
                }
            }
            return true;
        }
        else {
            alert("Cannot register since overbooking and waitinglist are false");
            return false
        }
    }
    else
        return true;
}

This function is almost similar to a pattern from this blog post. Coding Horror (yes this is the blog's name) named this pattern as arrow code. People tend to do this because it normal thing when we do programming, we add code, add a condition, add code again, add a condition then viola! You created the horror story.

Changes

Below is my suggestion for the above code:

function CheckEventBooking(eventMaintenanceInfo) {var data = eventMaintenanceInfo.results[0];var allowOverBooking = data.gent_AllowOverBooking;var targetedPaxBalance = data.gent_TargetedPAXBalance;var waitingList = data.gent_IsWaitListAllowed;var overBookingPax = data.gent_OverbookingPax;
if(!targetedPaxBalance || targetedPaxBalance !== 0) return true;var valid = allowOverBooking || waitingList;if(!valid) {	alert("Cannot register since overbooking and waitinglist are false");	return false;}
if (allowOverBooking && waitingList && overBookingPax > 0) {	var confirm = window.confirm("OverBooking & Waitinglist is allowed do you want to Confirm or Waitlist");	var result = confirm ? "Confirm" : "Waitinglist";
	Xrm.Page.getAttribute("gent_confirmation").setValue(result);}
return true;
}

With these fixes, you will see more readability code. You cut a lot of complexity (remove the if-else statement).

So how you think? Is it cleaner?