Tuesday, September 22, 2009

Multiple vs. Single Exit Points

Recently in one of the newsgroups a side discussion came up about the benefits of a single exit point from a function. There was a time when I followed that notion very closely. However, I found that the functions ended up having more nesting and other logic to "jump over" code paths once the return value was determined. In recent years I have found myself pretty much ignoring that idea completely.

Here is an example of some actual code I have somewhere. I have removed the null-checks, comments, and other exception throws to get down to the core of the logic.

In this snippet you can see that once the return value is determined, it immediately returns it.

    1 private int CompareTokens(CommandLineParser commandLineParser)
    2 {
    3     var n = System.Math.Min(this.tokens.Length, commandLineParser.tokens.Length);
    4 
    5     for (var i = 0; i < n; i++)
    6     {
    7         var result = this.tokens[i].CompareTo(commandLineParser.tokens[i]);
    8         if (result != 0)
    9         {
   10             return result;
   11         }
   12     }
   13 
   14     if (this.tokens.Length < commandLineParser.tokens.Length)
   15     {
   16         return -1;
   17     }
   18 
   19     if (this.tokens.Length > commandLineParser.tokens.Length)
   20     {
   21         return +1;
   22     }
   23 
   24     return 0;
   25 }

In the next snippet I have refactored the original routine into a single exit point. Note the required variable for the return value in line 3, the break at line 12 and the extra logic at lines 16 and 22 required to jump over the other code paths. Somewhere I know I have a more severe example. I will look for it. In fact, I remember it is an old one that used to have one exit point that I have since converted to multiple.

    1 private int CompareTokens(CommandLineParser commandLineParser)
    2 {
    3     var result = 0;
    4 
    5     var n = System.Math.Min(this.tokens.Length, commandLineParser.tokens.Length);
    6 
    7     for (var i = 0; i < n; i++)
    8     {
    9         result = this.tokens[i].CompareTo(commandLineParser.tokens[i]);
   10         if (result != 0)
   11         {
   12             break;
   13         }
   14     }
   15 
   16     if (result == 0)
   17     {
   18         if (this.tokens.Length < commandLineParser.tokens.Length)
   19         {
   20             result = -1;
   21         }
   22         else if (this.tokens.Length > commandLineParser.tokens.Length)
   23         {
   24             result = +1;
   25         }
   26     }
   27 
   28     return result;
   29 }

So, what's your opinion? I suppose some of you have seen coding horrors where multiple exit points have been abused.

No comments:

Post a Comment