I know how horrible and hard to read programs with goto
s are, and I'd like to avoid them at all costs, as everyone else.
However, I encountered a situation where I had a hard time avoiding them while having an elegant program. It is with error processing. I do a lot of steps, and if there is an error in any of the step, the program should display an error message, free some memory and return false
. If everything works, it should return true
.
The original program is like that :
bool init_routine()
{
int errcode; // Error code
errcode = do_someting();
if(errcode != RETURN_SUCCESS)
{
free_some_memory();
std::cerr << "Something went wrong in initialization.";
return false;
}
errcode = do_something_else();
if(errcode != RETURN_SUCCESS)
{
free_some_more_memory(); // Was allocated in the do_something function
free_some_memory();
std::cerr << "Something went wrong in initialization.";
return false;
}
/* The same pattern repeats quite a few times... */
return true; // Everything was ok
}
This is ugly, because some code is copy-pasted over and over, which is bad practice. I could use a function, but then the function cannot return false
from the callee, because C++ lacks what I'd call a double return instruction.
Another way to do it, is to use nested if
:
bool init_routine()
{
int errcode; // Error code
errcode = do_someting();
if(errcode == RETURN_SUCCESS)
{
errcode = do_something_else();
if(errcode == RETURN_SUCCESS)
{
/* The same pattern repeats quite a few times */
if()
{
if()
{
if()
{
return true;
}
}
}
}
free_some_more_memory();
}
free_some_memory();
std::cerr << "Something went wrong in initialization.";
return false;
}
This avoids any repetition and is compact. However, as the number of nested if
grows quickly, the indentation gets in the way. Moreover, all those if
hides the simple control flow of the program which just sequentially executes instructions and exits on errors, and makes the reader believe there is a lot of complicated conditions to follow.
Another way, which is in my opinion more natural, is :
bool init_routine()
{
int errcode; // Error code
errcode = do_someting();
if(errcode != RETURN_SUCCESS)
{
goto err1;
}
errcode = do_something_else();
if(errcode != RETURN_SUCCESS)
{
goto err2;
}
/* The same pattern repeats quite a few times... */
return true; // Everything was ok
err2:
free_some_more_memory();
err1:
free_some_memory();
std::cerr << "Something went wrong in initialization";
return false;
}
There is no repetition, the only problem is that the evil goto
is used. My question is :
Is there a way to not use goto
, while not using repetition, not using exceptions, and not having insane indentation ?
EDIT : Just to make it clear, I can't use exceptions because the routines I call are from an external library written in C, and I call them from my C++ code. Sorry for not mentioning that earlier.
goto
there are respected programmers who do use forward-gotos
in cases like this. Anyway, there is an alternative solution: create a one-shot loop.So your code can be refactored like this:
bool init_routine()
{
int errcode; // Error code
bool need_free_some_memory = false;
bool need_free_some_more_memory = false;
do { //not a loop, just scope for break
need_free_some_memory = true;
errcode = do_something();
if(errcode != RETURN_SUCCESS)
break;
need_free_some_more_memory = true;
errcode = do_something_else();
if(errcode != RETURN_SUCCESS)
break;
} while(0);
if(need_free_some_more_memory)
free_some_more_memory(); // Was allocated in the do_something function
if(need_free_some_memory)
free_some_memory();
if(errcode != RETURN_SUCCESS)
std::cerr << "Something went wrong in initialization.";
return errcode == RETURN_SUCCESS; // Everything was ok
}