Introduction

Many people who’ve witnessed me review code, whether it’s their code or not, know how I like my if statements. Without explicitly telling you what that is just yet, there’s a coding style for methods which results in code opposed to my preference. It states that:

Every method should have one and only one point of return.

I’m not sure where this guideline came from. I expect it has perfectly valid roots. However, in C#, I find constraining code to follow this guideline results in code which tends to be more difficult to read and maintain.

Example

Let’s suppose we have the following use case:

Jim’s been with the company for a while, and his user account has been given permission to kill. Jim selects a victim and clicks the kill button. A message appears asking Jim if he wants to continue with the kill operation with the options yes or no. Jim chooses yes. Jim is then presented with an area to type a message for his victim with the option to cancel or continue. Jim types a message and continues. A final message appears asking Jim to give a final confirmation with the options yes or no. Jim is sure he wants to kill, and so continues, rolling his eyes at the contrived and potentially offensive example he has just been a part of.

Let’s write an example of a single method implementing that use case following our trusty guideline. We’ll add some cancellation messages for things like permission denied etc:

Now how would I typically write that method?

Reading the First Example

I’ll start by stating how I read each example. The first one, following the single return point rule:

• If we’ve chosen a victim…
• …and Jim gives permission…
• …and Jim enters a message…
• …and Jim gives final confirmation…
• …do the kill…
• …otherwise, show the chicken message.
• And otherwise, hang on, ah, if we didn’t half permission we should show a permission denied message.
• Oh, and if we didn’t select a victim show a message.

I naturally read from top to bottom. The else blocks mean that operations which would normally be executed before the kill operation are actually written after the kill operation. This means that I have to refer back to the top of the method as I get past half way. I get so far, and end up having to refer back to the initial if blocks at the top in order to find out what the elses are doing.

In addition, the nesting of the ifs means I’m trying to track in my head all of the conditions that must be satisfied to get so far into the nesting. This is required so I know the conditions for reaching the else blocks. In fact, I probably wouldn’t remember all the conditions, and would have to read the method multiple times in order to understand all the paths. What’s more, the meat of the method is tucked away in the most inner if block which is indented 5 levels. That operation is the whole reason that method’s been called!

Reading the Second Example

Next up, how I read my example:

• If Jim didn’t select a victim then show a message and bail.
• If Jim doesn’t have permission, show a message and bail.
• If Jim fails to give confirmation, bail.
• If Jim doesn’t enter a message, bail.
• If Jim doesn’t give final confirmation, show a message and bail.
• If we got this far, then do the kill.

Here, each check is regarded as a corner case which is dealt with immediately. If we’re not satisfied, just bail with the minimum amount of work. I’m not tracking if conditions in my head, because once we’ve done a check and we’re still in the method, the condition no longer matters because I know it’s satisfied. What’s more, the method’s meat is right where it should be at indentation level 0. I can easily read and understand a method like this in one pass.

Other Points

The second version of the method is much easier to refactor. You can easily cut any of those if blocks out into its own method. Cutting a mid-level if in the first example would require restructuring.

Single points of return are something of a fallacy in C#. We have exceptions. If we chose to throw an ArgumentNullException if the victim is null rather than show a message, we’d have multiple return points in both our implementations. In fact, the possibility of a ThreadAbortException at any point means that code can return from (almost) anywhere.

Counterpoints

There are a few reasons I can think of for following this guideline in C#:

• You may want to port the code to a language without multiple points of return.
• It may encourage some developers to refactor nested if blocks into smaller methods, keeping the complexity of individual methods down.

The first point is fairly valid.

For the second point, you could follow the single point of return rule and end up with code that doesn’t creep across the screen by refactoring each if block into its own method. However, I think it’s pretty unlikely that the rule causes many to do that, and simply results in code written like the first example above. In that context it does much more harm than good.

Finally

When I have to change a method written like the first example above, one of the first steps I take is to slowly invert the if blocks to get something more linear and easier to comprehend. I’m then free to factor out some functionality into other methods if I think it’s necessary. By inverting if statements and returning early, the complexity remains the same, but I find it’s much easier to hold everything in my head.

In a sentence: if inversion can make potentially complex control flows easier to understand.