Search code examples
c++cgoto

Skip code without using state variable, using goto envisaged


I have a code which has parts that mustn't be executed if there was an error before in the code. I actually use a bool variable called EndProg that, if set to true, will instruct the program to avoid executing some parts of code.

My problem is that I don't want to use this method and I'd prefer to use goto instead because it will make the program jump to the cleanup part and avoid checking EndProg value multiple times.

The other problem is that I've read on many pages on StackOverflow and other websites that using goto is considered a bad practice and that it can make a code more difficult to read or create errors.

My code is simple enough and I will need to use just one label so I doubt that this will create problems; but I would like to know if there are other ways to do what I want without creating functions to do cleanup tasks or using return (because, for example, I will need to write the cleanup code several times) and I also don't want to write the same big cleanup code in multiple places and then use return or do something else.

I don't want to increase the number of lines of code nor use return nor use a lot of if nor check the value of a state variable. What would you recommend ?

Here's a piece of code :

bool EndProg=false;
/*
    Lot of code that can set EndProg to true
*/
ClassType ClassName;
if(!EndProg && LoadConf(&ConfFilePath,&ClassName)==0)
{
    int fildes=-1;
    if(ClassName.abc) // bool
    {
        if(ClassName.FilePath==0) // char *
        {
            ClassName.FilePath=new(std::nothrow) char[9]();
            if(ClassName.FilePath!=0)strcpy(ClassName.FilePath,"file.ext");
            else EndProg=true;
        }
        if(!EndProg && mkfifo(ClassName.FilePath,S_IRUSR | S_IWUSR)==-1)
        {
            if(errno==EEXIST)
            {
                /* EEXIST is returned if the file already exists
                We continue, later we will try to open this file */
            }
            else EndProg=true;
        }
        if(!EndProg && (fildes=open(ClassName.FilePath,O_RDWR))==-1)EndProg=true;
    }
    /*
    Lot of code that will check if EndProg == true
    */
}
delete[] ClassName.FilePath;
delete[] ConfFilePath;

What I would like to do is :

bool EndProg=false;
/*
    Lot of code that can set EndProg to true
*/
ClassType ClassName;
if(LoadConf(&ConfFilePath,&ClassName)==0)
{
    int fildes=-1;
    if(ClassName.abc) // bool
    {
        if(ClassName.FilePath==0) // char *
        {
            ClassName.FilePath=new(std::nothrow) char[9]();
            if(ClassName.FilePath==0)goto cleanup;
            strcpy(ClassName.FilePath,"file.ext");
        }
        if(mkfifo(ClassName.FilePath,S_IRUSR | S_IWUSR)==-1)
        {
            if(errno==EEXIST)
            {
                /* EEXIST is returned if the file already exists
                We continue, later we will try to open this file */
            }
            else goto cleanup;
        }
        if((fildes=open(ClassName.FilePath,O_RDWR))==-1)goto cleanup;
    }
    /*
    Lot of code that will check if EndProg == true
    */
}
cleanup:
delete[] ClassName.FilePath;
delete[] ConfFilePath;

As you can see it isn't difficult to understand and even if searching the label can be a problem for someone, it isn't for me; and I don't plan to make the code public.

Update :

I decided to using exceptions and it works for some parts of my original code. But I doubt this will be easy to implement in more complex parts. Thanks for your answers.


Solution

  • Since you've tagged this question c++ as well I'd go with using exceptions and a try catch block.

    You can find a lot of useful information about the subject on SO and on other websites:

    Here is a very basic tutorial.

    And here is a nice and basic FAQ that might help you as well.

    Basically there's nothing to fear, exceptions are not cryptic and in fact make more sense when you get the hang of it. Because basically this concept enables you to achieve exactly what you want:

    Several pitfalls that can be handled by the same error handling code.

    Edit: For example if I'd move the mkfifo etc. into a function (in general creating a function for each well defined logical block is clearer and more readable) and have something like

    This is just a sketch to give you a general idea:

    #include <exception>
    
    
    functionThatDoesMakeFifo(...){
       // check which ever conditions you want to check after mkfifo
       // if one of them goes wrong just do:
       throw std::exception();
    
    }
    
    // this is inside your function:
       ClassType ClassName;       
       try{
          ClassName.FilePath = new char[9](); // even though I'd use a string...
          .
          .
          . // rest of the code
       } catch(std::exception &e){    
           delete [] ClassName.FilePath;
           delete [] ConfFilePath;
           ClassName.FilePath = NULL; // not mandatory just my habit
           ConfFilePath = NULL;
       }