This is a common situation and there are many common ways to deal with it. Here's my attempt at a canonical answer. Please comment if I missed anything and I'll keep this post up to date.
This is an Arrow
What you are discussing is known as the arrow anti-pattern. It is called an arrow because the chain of nested ifs form code blocks that expand farther and farther to the right and then back to the left, forming a visual arrow that "points" to the right side of the code editor pane.
Flatten the Arrow with the Guard
Some common ways of avoiding the Arrow are discussed here. The most common method is to use a guard pattern, in which the code handles the exception flows first and then handles the basic flow, e.g. instead of
if (ok)
{
DoSomething();
}
else
{
_log.Error("oops");
return;
}
... you'd use....
if (!ok)
{
_log.Error("oops");
return;
}
DoSomething(); //notice how this is already farther to the left than the example above
When there is a long series of guards this flattens the code considerably as all the guards appear all the way to the left and your ifs are not nested. In addition, you are visually pairing the logic condition with its associated error, which makes it far easier to tell what is going on:
Arrow:
ok = DoSomething1();
if (ok)
{
ok = DoSomething2();
if (ok)
{
ok = DoSomething3();
if (!ok)
{
_log.Error("oops"); //Tip of the Arrow
return;
}
}
else
{
_log.Error("oops");
return;
}
}
else
{
_log.Error("oops");
return;
}
Guard:
ok = DoSomething1();
if (!ok)
{
_log.Error("oops");
return;
}
ok = DoSomething2();
if (!ok)
{
_log.Error("oops");
return;
}
ok = DoSomething3();
if (!ok)
{
_log.Error("oops");
return;
}
ok = DoSomething4();
if (!ok)
{
_log.Error("oops");
return;
}
This is objectively and quantifiably easier to read because
- The { and } characters for a given logic block are closer together
- The amount of mental context needed to understand a particular line is smaller
- The entirety of logic associated with an if condition is more likely to be on one page
- The need for the coder to scroll the page/eye track is greatly lessened
How to add common code at the end
The problem with the guard pattern is that it relies on what is called "opportunistic return" or "opportunistic exit." In other words, it breaks the pattern that each and every function should have exactly one point of exit. This is a problem for two reasons:
- It rubs some people the wrong way, e.g. people who learned to code on Pascal have learned that one function = one exit point.
- It does not provide a section of code that executes upon exit no matter what, which is the subject at hand.
Below I've provided some options for working around this limitation either by using language features or by avoiding the problem altogether.
Option 1. You can't do this: use finally
Unfortunately, as a c++ developer, you can't do this. But this is the number one answer for languages that contain a finally keyword, since this is exactly what it is for.
try
{
if (!ok)
{
_log.Error("oops");
return;
}
DoSomething(); //notice how this is already farther to the left than the example above
}
finally
{
DoSomethingNoMatterWhat();
}
Option 2. Avoid the issue: Restructure your functions
You can avoid the problem by breaking the code into two functions. This solution has the benefit of working for any language, and additionally it can reduce cyclomatic complexity, which is a proven way to reduce your defect rate, and improves the specificity of any automated unit tests.
Here's an example:
void OuterFunction()
{
DoSomethingIfPossible();
DoSomethingNoMatterWhat();
}
void DoSomethingIfPossible()
{
if (!ok)
{
_log.Error("Oops");
return;
}
DoSomething();
}
Option 3. Language trick: Use a fake loop
Another common trick I see is using while(true) and break, as shown in the other answers.
while(true)
{
if (!ok) break;
DoSomething();
break; //important
}
DoSomethingNoMatterWhat();
While this is less "honest" than using goto, it is less prone to being messed up when refactoring, as it clearly marks the boundaries of logic scope. A naive coder who cuts and pastes your labels or your goto statements can cause major problems! (And frankly the pattern is so common now I think it clearly communicates the intent, and is therefore not "dishonest" at all).
There are other variants of this options. For example, one could use switch instead of while. Any language construct with a break keyword would probably work.
Option 4. Leverage the object life cycle
One other approach leverages the object life cycle. Use a context object to carry around your parameters (something which our naive example suspiciously lacks) and dispose of it when you're done.
class MyContext
{
~MyContext()
{
DoSomethingNoMatterWhat();
}
}
void MainMethod()
{
MyContext myContext;
ok = DoSomething(myContext);
if (!ok)
{
_log.Error("Oops");
return;
}
ok = DoSomethingElse(myContext);
if (!ok)
{
_log.Error("Oops");
return;
}
ok = DoSomethingMore(myContext);
if (!ok)
{
_log.Error("Oops");
}
//DoSomethingNoMatterWhat will be called when myContext goes out of scope
}
Note: Be sure you understand the object life cycle of your language of choice. You need some sort of deterministic garbage collection for this to work, i.e. you have to know when the destructor will be called. In some languages you will need to use Dispose instead of a destructor.
Option 4.1. Leverage the object life cycle (wrapper pattern)
If you're going to use an object-oriented approach, may as well do it right. This option uses a class to "wrap" the resources that require cleanup, as well as its other operations.
class MyWrapper
{
bool DoSomething() {...};
bool DoSomethingElse() {...}
void ~MyWapper()
{
DoSomethingNoMatterWhat();
}
}
void MainMethod()
{
bool ok = myWrapper.DoSomething();
if (!ok)
_log.Error("Oops");
return;
}
ok = myWrapper.DoSomethingElse();
if (!ok)
_log.Error("Oops");
return;
}
}
//DoSomethingNoMatterWhat will be called when myWrapper is destroyed
Again, be sure you understand your object life cycle.
Option 5. Language trick: Use short-circuit evaluation
Another technique is to take advantage of short-circuit evaluation.
if (DoSomething1() && DoSomething2() && DoSomething3())
{
DoSomething4();
}
DoSomethingNoMatterWhat();
This solution takes advantage of the way the && operator works. When the left hand side of && evaluates to false, the right hand side is never evaluated.
This trick is most useful when compact code is required and when the code is not likely to see much maintenance, e.g you are implementing a well-known algorithm. For more general coding the structure of this code is too brittle; even a minor change to the logic could trigger a total rewrite.
ifstatement:if( execA() && execB() && ... )– Michaelfalsebe considered as akin to an exceptional situation? – Machinatefalsereturn can be pretty normal. – Corrosivefalseis a normal condition. – Journalifstatements. Just to avoid somebody now saying "You could remove your temporary dir after...bla bla bla" – Journalstd::function. Andrei's stuff is more industrial strength library implementation. I'm not so sure that it's better then the simple trivial thing, though, because simplicity is important in itself, as I see it! ;-) – Churchlygoto. (Or, you can create a new function and use the very relatedreturn.) – Afiretry-throw-catch), which could be the appropriate solution for this problem using that language? – Touchwood