About Cyclomatic Complexity

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

So just today I check my email and see there is an automatic email contain changeset from one of my co-worker 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 one because it normal things when we do programming, we add code, add a condition, add code again, add condition then viola! You created the horror story.

Changes

Below is my suggestion for 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 if-else statement).

So how you think? Is it cleaner?

Advertisement