Which method is more common/accepted for implementing this type function?

I have a very basic function. There are numerous ways of implementing this.
I'm curious to know which method people prefer to use the most? And for programming etiquette, which one should be used? I'm guilty of using all 4 styles, but I'm more inclined to use the last method. The reason is the bool can be used over again in the same function for debugging. Also very last statement in the function is a return value.
I always wonder if there actually was a right and wrong way of doing this.
(I don't care about execution time for this example)

Code (csharp):

1.  
2. //Method 1
3. private bool IsItemInTunnel(Object tunnel)
4.    {
5.      if (tunnel.tunnelAreaBounds.bounds.Contains(position))
6.      {
7.        return true;
8.      }
9.      return false;
10.    }
11.  
12. //Method 2
13.    private bool IsItemInTunnel(Object tunnel)
14.    {
15.      if (tunnel.tunnelAreaBounds.bounds.Contains(position))
16.      {
17.        return true;
18.      }
19.      else
20.      {
21.        return false;
22.      }
23.    }
24.  
25. //Method 3
26.    private bool  IsItemInTunnel(Object tunnel)
27.    {
28.      return tunnel.tunnelAreaBounds.bounds.Contains(position);
29.    }
30.  
31. //Method 4
32.    private bool IsItemInTunnel(Object tunnel)
33.    {
34.      bool isInTunnel = tunnel.tunnelAreaBounds.bounds.Contains(position);
35.  
36.      return isInTunnel ;
37.    }
38.  

What do people think?

 

It really depends, but 3 or 4 would generally be the choices I'd make. 3 is good if you just need a check. 4 if you need to check and then plan to do something else with that check, but then you might not be returning a bool in 4. Maybe you're checking if something, then return a different amount of gold for example.

 

I usually do #1 or #4. For #3, it's too hard to add additional statement to the function, like if you needed to debug something, it's pretty much locked-in. For #2, I consider the code is longer than it needs to be. For #1 and #2, if the code grew in length, the first return statement might become buried in the code and not be obvious that the function is "returning early". I've read some programming guidelines for some languages, and they say a function should only have one "return" statement per function, to avoid multiple return statements when possible. This is why I tend to use #4.

 

Use #3. Don't have code looking like #4 "in case" you need to put more statements there. Changing from 3 to 4 is not a lot of work! Even less if you have an editor that's not bad and allows for extracting local variables with a shortcut.

For 1 and 2, I really don't like code that boils down to:

Code (csharp):

1. if(bool) {
2.     return bool;
3. }
4. else {
5.     return !bool;
6. }

To me, it imples that the writer is not comfortable with booleans. You really should be!

 

Noticed that Microsoft uses method #4 in the usage example of a return statement in C#:
https://msdn.microsoft.com/en-us/library/1h3swy84.aspx

Uses method #4 for JScript:
https://msdn.microsoft.com/en-us/library/s3ysy6h9(v=vs.100).aspx

For C++, it uses method #3
https://msdn.microsoft.com/en-us/library/k68ktdwf.aspx

 

Often tutorials use an approach that is not necessarily optimal or "the normal way" because it makes it easier to learn.

I fully agree with Baste, #3 is how I'd write it and is what I'd expect to see in any code by somebody I'd hire or work with.

#3 is also the easiest to read.

 

Who really cares? Just use whichever one you typed first. This is one that's probably not worth the time it takes to think about.

Number 3 would be the most performant, by about half a picosecond. It also requires a couple less keystrokes. So in the absence of any real reason to make a choice, I would default to number 3.

If you need side effects, its trivially easy to convert between the two different types of methods.

 

I'd care if I were looking at someone else's code and saw something like #1 or #2... I'd wonder what the heck he was thinking and start watching what he does in case it warranted passing a recommendation up the line that we need a replacement. I work with a lot of lowest-bidder offshore guys hired by HR people who don't know a switch statement from a DVD burner, so the few remaining senior devs are often stuck running interference this way.

I figure any opportunity to encourage somebody to do things "the right way" is worthwhile. If somebody cares enough to ask, I'm happy to burn a minute or two lending a hand. It's more interest than we usually get out of our paid contractors.

 

Number 3 is the most efficient in terms of style... However, if you were to ask someone like Linus Torvald he would probably rage because you didn't just use "if (tunnel.tunnelAreaBounds.bounds.Contains(someObject.position)) { //doSomething(); }" where applicable.

P.S. If you were looking for maximum performance there are much better ways to implement an "IsInTunnel" flag.

 

It very much matters. Coming from a DoD background, I can tell you that #4 would be the only acceptable way for the project we were working on. This were the requirements for flight controller software (I cant remember the exact wording):
The function must only have one return statement.
The value being returned must be declared and stored in a parameter inside the function
The function will only return the value stored inside the local parameter.

This is coming from flight controller software requirements. So yeah, it matters in some cases depending on who is paying you.

 

@fg34 your post leads me to wonder how the various lint utilities out there would respond to the four code examples. I don't really use them but doesn't Visual Studio automatically install something these days? I vaguely recall there are some freebies available through NuGet, too.

 

I like #4 because it makes it simple to inspect and modify the result of tunnel.tunnelAreaBounds.bounds.Contains(position); before returning from your function when you are in debug mode.

A good compiler will reduce #3 and #4 to identical IL when built in release mode, so I have no concern about micro-optimizations.

Multiple returns are okay if they improve readability. The "guard statement" is a great example.