Branching Logic vs. Guard Logic When It Comes To Function Control Flow

So what would you do in the case of three possible exits?

Would you do:

<cfif x = y>
<cfreturn 1>
</cfif>

<cfif x = z>
<cfreturn 2>
</cfif>

<cfreturn 3>

As usual, Ben, some food for thought!

I must confess that I have used both in the past without actually drawing a mental distinction between them.

But I do like the concept of the guard-logic, which apart from anything else can do away with many nested if/else statements. Although if you have that many nested if/else statements, a bit of refactoring is probably in order anyway...

@Jessie - cfswitch with defaultcase?

Maybe it is due to the simplistic examples provided, but I am having trouble finding either scenario to be more advantageous than the other. To me, they both read very similar. I am sure this is a very subjective topic though and many will prefer one over the other.

I sometimes find multiple exit points, especially when heavily nested, to be confusing. If possible, I try to use a single exit point at the end of the function and leverage variables to determine how to exit.

For example:

<cffunction name="doSomething">
<cfargument name="style" />

<cfset var result = false />

<cfif (arguments.style eq "guard")>

<cfset result = true />

</cfif>

<cfreturn result />

</cffunction>

Ben, good stuff. For "branching" how about "procedural"? I was given a lecture by a bad mannered client with a computing degree, and this is the only part I remember.

@Scott,

Usually guard clauses are used when the function does normally does something non-trivial, but for some inputs a trivial result can be returned.

<cffunction name="sluggingPercentage">

<cfif variables.cachedsluggingPercentage neq "">
<cfreturn cachedsluggingPercentage>
</cfif>

<cfif arrayLen(variables.atBats) eq 0>
<cfreturn 0>
</cfif>

<!---
This part shall be left to the imagination, as it was during
my childhood, before the internet ruined everything.
--->

<cfreturn computedSluggingPercentage>

</cfif>

Ben,

Like Seb, I have used both styles - probably without quite enough thought going into it.

I tend to prefer the "branching" style based on the philosophy that a method should have one and only one exit point. Then I will use the "guard" style only when the branching leads to excessive complexity or I really want to make the "guard" nature very clear.

All of that being said, I like your idea of putting terms to these different approaches to make the decision a little easier to see and conceptualize.

Not only that, but I think this makes an argument that I have likely been to strict in my "a function should only have a single return" philosophy.

One of the places I find "Guard Logic" to be useful is in my Model-Glue apps. In Model-Glue, if a certain condition is met, you can issue a statement in your controller function to add a "result" to the event object. That result ends up acting like a return statement/interrupt that skips over any of the other message broadcasts (which are calls to other controller functions to execute model code) in that event handler and goes straight to the result code. It's a nice way of avoiding unnecessary code execution when you've already reached a success or failure condition.

I guess it's fine as long as your function is short. However, multiple return points are not encouraged when I was in college. They prefer a short if block for guards and long if block for actual "Branching Logic", and then follow by a single return statement.

As an old COBOL hack, I do miss the GOTO statement... (ducks the flying bricks) .. where you had to exit at the end of a piece of PERFORMED code (at least in the version I was using).

I tend to write functions that sit in a cftry block which then falls through to the single cfreturn at the end - sort of like....

<cffunction name="myfunction">
<cfset var LOCALS={}>
<cfset LOCALS.ret.status="OK">
<cfset LOCALS.ret.message="">
<cftry>
 
	Do something worthwhile here ....
 
	<cfif an error occurs >
		<cfthrow message="It went wrong">
	</cfif>
 
	<cfset LOCALS.ret.returnvalue="returnvalue">
 
	<cfcatch>
		<cfset LOCALS.status="FAIL">
		<cfset LOCALS.message="Oh crap">
	</cfcatch>
	<cfreturn LOCALS.ret>
</cftry>

Using a struct in a standard format for all non-trivial functions lets me pass a status value and message back and makes it really easy to bubble-up error conditions.

It also makes the logic really easy to read as exception conditions are just 'thrown' out as they occur, leaving what amounts to a pretty liniar, easy to understand flow in the rest of the function.

I guess this is the 'guard logic' model, but with the added benefit of a bail-out which you can nevertheless guarantee passes through a single 'exit' point.

Like you, I prefer what you're calling guard logic.

I REALLY like how it keeps indention down.

Although it's fallen into disfavor in recent years, another advantage of guard logic is that you can often get away with not coding braces on an immediate return:

<input ... onchange="
if (validTaxId('SSN', this))
return true;// Allows reuse in onsubmit.
this.focus();
return false;  // Allows reuse in onsubmit.
">

Most folks nowadays would code

<input ... onchange="
if (validTaxId('SSN', this)) {
return true;// Allows reuse in onsubmit.
}
this.focus();
return false;  // Allows reuse in onsubmit.
">

The purpose of braces is to group together MULTIPLE statements, but it seems like religion nowadays that we have to always code braces, just in case we want to add more statements in the future, I guess.

But even the most code-like-me-or-you're-uncool firebrands seem to tolerate a braceless break, continue or return as the only statement following an if.

And this is the same thing (no braces), but now you'll REALLY think I'm crazy:

<form ... onsubmit="
if (!this.SSN       .onchange()) return false;
if (!this.FirstName .onchange()) return false;
if (!this.MiddleInit.onchange()) return false;
...
return true;
">

I hope the code tag lines up columns the way I did when I wrote it. Isn't it REALLY easy to read? You know exactly what's going on in a glance.

*sigh* I'll get the hang of the code tag here someday, I *promise*.

3 spaces were truncated before the first 2 "return true" statements.

6 spaces were truncated before the first .onchange() in the onsubmit.

I call this Short Circuit programming, as you're short circuiting the rest of the logic and taking the quickest route out of the method.
I've been using it for years and it creates VERY fast and readable code. Additionally, there are branching limitations. I've come across nasty validation logic code that consists of a ton of nested if/else statements. Coldfusion (actually java) returns an error about something to do with exceeding small int branching branching paths 128 after an upgrade to CF8. I presume it was using a signed byte to count how many branching paths, and when it reaches 128, it's just too complex. Most of that code got broken into discreet functions and short circuiting logic. Speed also drastically improved as well.

@Ben,

I also use it in conjunction with formatted structs as a lazy (nay, Creative and Efficient!) way to bubble up nested error messages without relying on try catch when I don't need to.

crude example:

inside component:

<cffunction name="getUserData" access="public" returntype="struct" output="false">
	<cfargument name="ID" type="numeric" required="true" />
	 
	<cfscript>
		var MyStruct = StructNew();
		var SomeDataStruct = StructNew();
		 
		MyStruct.Success = false;
		 
		SomeDataStruct = getSomeData(Arguments.ID);
		if(Not SomeDataStruct.Success)
			{ return SomeDataStruct; }
			 
		MyStruct.CurrentTime = Now();
		MyStruct.Email = SomeDataStruct.Email;
 
		MyStruct.Success = true;
 
		return MyStruct;
	</cfscript>
</cffunction>
<cffunction name="getSomeData" access="public" returntype="struct" output="false">
	<cfargument name="ID" type="numeric" required="true" />
 
	<cfscript>
		var MyStruct = StructNew();
		var qGetUser = '';
		 
		MyStruct.Success = false;
	</cfscript>
	<cfquery name="qGetUser" datasource="someDS">
		Select			*
		From			Users
		Where			User_ID = <cfqueryparam cfsqltype="cf_sql_integer" value="#Arguments.ID" />
	</cfquery>
	<cfscript>
		if(Not qGetUser.RecordCount)
		{
			MyStruct.ErrorComponent = "MyComponent";
			MyStruct.ErrorMethod = "getSomeData()";
			MyStruct.ErrorType = "DBLookup";
			MyStruct.ErrorMsg = "No Records Found";
			 
			return MyStruct;
		}
		 
		MyStruct.Username = qGetUser.Username[1];
		MyStruct.Email = qGetUser.Email[1];
		 
		MyStruct.Success = true;
		 
		return MyStruct;
	</cfscript>
</cffunction>

inside cfm:

<cfscript>
	variables.GetMyData = getUserData(1);
</cfscript>
<cfoutput>
	<cfif Not GetMyData.Success>
		An Error Occured. <br />
		Component: #variables.GetMyData.ErrorComponent#<br />
		Method: #variables.GetMyData.ErrorMethod#<br />
		Type: #variables.GetMyData.ErrorType#<br />
		Msg: #variables.GetMyData.ErrorMsg#<br />
		<cfif StructKeyExists(variables.GetMyData, "DBQuery")>
			Query: #variables.GetMyData.DBQuery#<br />
		</cfif>
	<cfelse>
		Time: variables.GetMyData.CurrentTime<br />
		Email: variables.GetMyData.Email<br />
	</cfif>
</cfoutput>

The 'universal' error keys in the struct only exist if Success is false (and no data will be present other than the error keys. don't want partial data. Either success, or nothing). If success is true, the data keys i expect I know will be present.

@Ben,

A more practical example:

<cfcomponent output="false">
	<cffunction name="Init" access="public" returntype="Authentication">
	 
		<cfreturn this />
	</cffunction>
 
	<cffunction name="login" access="public" returntype="struct" output="false">
		<cfargument name="Credentials" type="struct" required="true" />
 
		<cfscript>
			var MyStruct = StructNew();
			var qLogin = '';
 
			MyStruct.Success = false;
 
			if(Not StructKeyExists(Arguments.Credentials, "Username"))
			{
				MyStruct.ErrorComponent = "Authentication";
				MyStruct.ErrorMethod = "login()";
				MyStruct.ErrorType = "InvalidArgument";
				MyStruct.ErrorMsg = "'Username' is not present in struct";
 
				return MyStruct;
			}
 
			if(Not StructKeyExists(Arguments.Credentials, "Password"))
			{
				MyStruct.ErrorComponent = "Authentication";
				MyStruct.ErrorMethod = "login()";
				MyStruct.ErrorType = "InvalidArgument";
				MyStruct.ErrorMsg = "'Password' is not present in struct";
 
				return MyStruct;
			}
 
			qLogin = getLogin(Arguments.Credentials.Username);
			if(Not qLogin.RecordCount)
			{
				MyStruct.ErrorComponent = "Authentication";
				MyStruct.ErrorMethod = "login()";
				MyStruct.ErrorType = "DBLookup";
				MyStruct.ErrorMsg = "Unable to find User";
 
				return MyStruct;
			}
 
			if(Not Compare(qLogin.Hash[1], Hash(Arguments.Credentials.Password & qLogin.Salt[1],'SHA-512','UTF-8')))
			{
				MyStruct.ErrorComponent = "Authentication";
				MyStruct.ErrorMethod = "login()";
				MyStruct.ErrorType = "AuthPassMismatch";
				MyStruct.ErrorMsg = "Password does not match";
 
				return MyStruct;
			}
 
			MyStruct.ID = qLogin.User_ID[1];
			MyStruct.Username = qLogin.Username[1];
			MyStruct.Email = qLogin.Email[1];
 
			MyStruct.Success = true;
 
			return Mystruct;
		</cfscript>
	</cffunction>
 
	<cffunction name="getLogin" access="private" returntype="query" output="false">
		<cfargument name="Username" type="string" required="true" />
		 
		<cfscript>
			var qLogin = '';
		</cfscript>
		<cfquery name="qLogin" datasource="myDS">
			Select			User_ID,
							Username,
							Hash,
							Salt,
							Email
			From			Users
			Where			Username = <cfqueryparam cfsqltype="cf_sql_varchar" value="Arguments.Username">
		</cfquery>
		<cfreturn qLogin />
	</cffunction>
</cfcomponent>

I really hate seeing functions where the entire body of the function is inside an if statement. I always write what you're calling guard logic here. It reduces the noise in the function and means there's less indentation.

It's also better to structure your function in terms of lots if if statements that have returns and no else because you can easily find code duplication and refactor for AOP style advice later.

ex.

function example(...) {
if (...) return true;
if (...) return true;
// compute something
return computation;
}

Both of those return statements might be candidates for refactoring into advice. If we had a bunch of nested if statements that'd be harder to notice though.

@Scott: Using a single exit point is not always that good, its not only about readability, its also about performance. If you know a situation should get out of the function right from the beginning, you should do so instead of letting it continue all the way down and go through all the next statements, cause thats just useless and a waste of time. I always use the guard logic, not because I read about it, it's just logic to me; one main stream down and whathever doesn't follow, get out right away.

I know you tackled this question in ColdFusion, but I just had a revelation that this could kill JS performance and jsperf confirmed it: http://twitter.com/#!/oneguystudio/status/84091367088852992

For anyone not following on twitter...

Drew's test has a bug and calls Math.random() twice in the early return. If you fix that you'll see both are nearly identical in performance. The test is really flawed either way since the slowness of Math.random() is drowning out any difference from the early return/assignments.

Note that a decent JS compiler may rewrite the entire function to remove the early return (or add one). And that all modern browsers are going to turn both into very efficient assembly code when the JIT kicks in. Language constructs have very little performance impact (except "with" which turns off optimizations and makes everything slow...)

@Elliott,

Math.random() probably is affecting the performance, but so is the function call itself. Branching logic has a major impact on performance this was just not a good test of it. A true test would remove the function call as well.

@Drew

I'd challenge you to show me real world examples where branching logic has any statistical difference.

You're fighting the bare metal here. The assembly generated by the VM is going to be so fast that branching logic (CMP) and return (RET) likely have almost no impact at all. You might as well write this test in C.

@Elliott,

@Elliott,

I wanted to test returns inside a function, I can't test those without paying the cost of opening a function :P. So obviously, calling a method costs far more than premature returns 'guarded logic' inside it.

Of course, some code that requires no if/then/else would be faster than some that did, so thats not relevant. If the CPU can just burn through the numbers, it would outpace having to make decisions along the way. Again, this isn't what I was looking into, I wanted to see if returning early had an impact on performance.

@Drew Your test is flawed. When doing short circuiting logic, you typically do such in a function with a high amount of comparisons such as validation of a whole form. In this case, the first instance you fail and return early, then better as you jump over comparisons that don't need to be made. When your method returns an object, you will have some default state of the object, only filling out and returning a more complete object later in the method if you need to get that far. Your test doesn't do nearly enough work to be a valid comparison.