Search code examples
c++error-handlinggoto

How avoid goto in error processing


I know how horrible and hard to read programs with gotos 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.


Solution

    1. While I don't use 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.
    2. I also agree with the other guys, use RAII if possible. However in certain situations (e.g. legacy do_something()) you can do some 'manual-RAII' with boolean flags.

    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
    }