1 clear exit

[Peter Morris]

Code before:

private void RemoveCharacter(string characterName)
{
disableEvents = true;
bool found = false;
try
{
for (int catCount = 0; !found && (catCount < treeView.Nodes.Count);
catCount++)
{
for (int charCount = 0; !found && (charCount <
treeView.Nodes[catCount].Nodes.Count); charCount++)
{
if (some condition is met)
{
found = true;
//Some code here
}
}
}
}
finally
{
disableEvents = false;
}
}


Code after:
private void RemoveCharacter(string characterName)
{
disableEvents = true;
try
{
foreach (TreeNode categoryNode in treeView.Nodes)
foreach (TreeNode characterNode in categoryNode.Nodes)
if (some condition is met)
{
//Some code here
return;
}
}
finally
{
disableEvents = false;
}
}


The "before" argument is that there is only 1 point of exit. The "after"
argument is that it is (in my opinion) easier to read.

From being a Pascal coder I used to employe the "1 point of exit" myself,
but since switching to C# and using the return statement I don't mind using
multiple exit points as long as the code isn't too complicated to
understand. These days I don't agree with the "1 point of exit" strategy
because I think it makes the code harder to read and provides no benefit.


What do you guys think?

 

[Marc Bernard]

These days I don't agree with the "1 point of exit" strategy
because I think it makes the code harder to read and provides no benefit.

What do you guys think?

I favour readability over complexity, for sure. Too often developers
produce code that follows the "textbook" approach, but is unreadable.
You don't have to go "all in" - pick and choose what works best,
balanced with what is maintainable later on.

Marc
http://nomagichere.blogspot.com

 

[Pavel Minaev]

The "before" argument is that there is only 1 point of exit.  The "after"
argument is that it is (in my opinion) easier to read.

From being a Pascal coder I used to employe the "1 point of exit" myself,
but since switching to C# and using the return statement I don't mind using
multiple exit points as long as the code isn't too complicated to
understand.  These days I don't agree with the "1 point of exit" strategy
because I think it makes the code harder to read and provides no benefit.

What do you guys think?

I appreciate the theory behind "single point of exit" approach, but it
doesn't sit well with most imperative languages out there. In
practice, I think that it can be useful if practiced in moderation.
Returns from within 3-4 nested loops tend to mess up things for sure.
But a typical pattern of repeating if-return statements on a single
nesting level is much more readable than a mess of nested if's:

if (arg1 == null) return;
if (arg2 < arg3) return;
...
// actual logic starts here

For the specific case that you give, it is honestly hard to tell. If
it's the only return within the function body, then I would probably
say it's alright even despite the nesting.

To share a related anecdote... we all know that "goto is evil" etc.
But it is entertaining to observe the lengths to which some people are
willing to go to achieve precisely the same effect without mentioning
the dreaded word! My favorite anti-pattern along these lines is this:

do {
...
if (something_else_happened) break; // skip the rest
...
} while (false); // not really a loop...

A colleague of mine was quite fond of writing such things. When I
pointed out to him that he is in effect writing deliberately confusing
code by using a loop construct for something that is not a loop, and
hiding that logic in the middle of that pseudo-loop, he asked what
alternative there was. His reaction to my suggestion to just use
"goto", as it makes the plain transfer of control pattern much more
clear and explicit in this case, was met with revulsion: "but, but, it
is evil!". I did not manage to convince him that what he was doing
amounted to precisely the same thing anyway, and, goto or not,
"spaghetti code" was perfectly applicable.

And so arcane taboos that are followed but not understood are
born...

 

[Jeff Johnson]

It's not that it doesn't "sit well", IMHO. It's that, just as with your
example of using "goto", it's important to know when a general rule of
thumb should be followed, and when it's valid to violate the rule of
thumb.

There are plenty of situations where having a single point of exit makes
the code simpler to maintain. But a) in a language with a nice
"try/finally" syntax, it's easier to address the usual motivation for
having a single point of exit, and b) as soon as having a single point of
exit makes the code _harder_ to maintain, the entire point of the rule of
thumb is lost, and so it should be violated.

I love how this group is populated by sane, level-headed people.

(Of course, "sane level-headed people" = "people who think like I do.")

 

[Peter Morris]

in a language with a nice "try/finally" syntax, it's easier to address

the usual motivation for having a single point of exit

Actually that's a good point. The single-exit-point idea was thought up
long before try..finally blocks.