To GOTO or not to GOTO?

Question

Currently I am working on a project where goto statements are heavely used. The main purpose of goto statements is to have one cleanup section in a routine rather than multiple return statements. Like below:

BOOL foo()
{
   BOOL bRetVal = FALSE;
   int *p = NULL;

   p = new int;
   if (p == NULL)
   {
     cout<<" OOM \n";
     goto Exit;
   }

   // Lot of code...

Exit:
   if(p)
   {
     delete p;
     p = NULL;
   }
   return bRetVal;
}

This makes it much easier as we can track our clean up code at one section in code, that is, after the Exit label.

However, I have read many places it's bad practice to have goto statements.

Currently I am reading the Code Complete book, and it says that we need to use variables close to their declarations. If we use goto then we need to declare/initialize all variables before first use of goto otherwise the compiler will give errors that initialization of xx variable is skipped by the goto statement.

Which way is right?

---

From Scott's comment:

It looks like using goto to jump from one section to another is bad as it makes the code hard to read and understand.

But if we use goto just to go forward and to one label then it should be fine(?).

Answer 1

I am not sure what do you mean by clean up code but in C++ there is a concept called "resource acquisition is initialization" and it should be the responsibility of your destructors to clean up stuff.

(Note that in C# and Java, this is usually solved by try/finally)

For more info check out this page: http://www.research.att.com/~bs/bs_faq2.html#finally

EDIT: Let me clear this up a little bit.

Consider the following code:

void MyMethod()
{
    MyClass *myInstance = new MyClass("myParameter");
    /* Your code here */
    delete myInstance;
}

The problem: What happens if you have multiple exits from the function? You have to keep track of each exit and delete your objects at all possible exits! Otherwise, you will have memory leaks and zombie resources, right?

The solution: Use object references instead, as they get cleaned up automatically when the control leaves the scope.

void MyMethod()
{
    MyClass myInstance("myParameter");
    /* Your code here */
    /* You don't need delete - myInstance will be destructed and deleted
     * automatically on function exit */
}

Oh yes, and use std::unique_ptr or something similar because the example above as it is is obviously imperfect.