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?

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s

This site uses Akismet to reduce spam. Learn how your comment data is processed.