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