What is the proper use of dynamic allocation -- should this be avoided? (using new and delete in separate scopes)

Question

I am having trouble understanding proper usage of the new keyword. My question is:

1. Is the following just bad design as I suspect?
2. If not, where should I call delete?
3. If so, what is the better practice?

#include <string>
#include <iostream>

struct myOptions {
    int myInt;
    std::string myString;
};

myOptions* packageOptions() {
    myOptions* options = new myOptions;
    options->myInt = 42;
    options->myString = "hello world";

    return options;
}

int main() {
    myOptions* options = packageOptions();

    std::cout << options->myString << std::endl;
    std::cout << options->myInt << std::endl;

    delete myOptions; // this just feels wrong to put here
}

My gut is telling me it's bad because the main function shouldn't have to manage the memory allocated by other functions, as in it's breaking some kind of encapsulation. I thought about doing a class constructor/deconstructor, but that seems to be overkill.

Answer 1

What you have demonstrated is correct as far as managing the memory goes. But stylistically and from a maintenance point of view, it could be much better.

Two ways that are easier to maintain are:

1. using smart pointers
2. Redesigning your code so that you pass in a structure to optionsPackage, allowing the function to populate the passed in structure, and the called to worry about the lifespan and usage of the structure.

Smart pointers were created to help prevent memory leaks, which can be a long term pain to find in large projects. Look into std::shared_ptr or see the shared pointers suggested answers.

Passing in the actual structure allows the caller to worry about the memory space used, including allocating and deallocating it. This was the preferred way, before smart pointers came along, and is still the rule of thumb (letting the code that needs the data manage the data object associated with it).

Passing in the actual structure would make the code look something like this:

#include <string>
#include <iostream>

struct myOptions {
    int myInt;
    std::string myString;
};


void packageOptions( myOptions& theInputOptions) {
    theInputOptions.myInt = 42;
    theInputOptions.myString = "hello world";
}


int main() {
    myOptions options;
    packageOptions( options );

    std::cout << options.myString << std::endl;
    std::cout << options.myInt << std::endl;

}

I find this style far easier to maintain the code. It can be combined with smart pointers where needed.

Answer 2

Instead of using structures in this manner, try to use classes and constructors/destructors for the same.

Answer 3

Using shared_ptr is not the best option in your present case. I give this example to show you it exists, and then you don't have to bother about who/when the object is deleted. The shared_ptr will call the destructor/delete whenever there are no references on the object left, ie at the end of main in your example. (also shared_ptr has been introduced in C++11, not available in C++03)

#include <string>
#include <iostream>
#include <memory>

struct myOptions {
    int myInt;
    std::string myString;
};

using OptionsPtr = std::shared_ptr<myOptions>;

OptionsPtr packageOptions() {
    OptionsPtr options = std::make_shared<myOptions>();
    options->myInt = 42;
    options->myString = "hello world";

    return options;
}

int main() {
    OptionsPtr options = packageOptions();

    std::cout << options->myString << std::endl;
    std::cout << options->myInt << std::endl;
}

Anyway, in your case, stack allocation, like in @Cyber answer, is much preferable.

Answer 4

From a strictly technical perspective, what you are doing is fine.

But you asked what the proper use of new is [in C++] -- and the answer may surprise you.

The most proper way to use new is not to. The same can be said of delete. Instead of using new/delete, you should be using smart pointers such as std::shared_ptr along with an accompanying make_shared.

I'm not saying there are no exceptions, but those exceptions would be unusual and typically the result of a design that could be otherwise modified.

In fact, I would also suggest another question altogether: what is the proper use of dynamic allocation? Again, the answer I would suggest is don't use dynamic allocation. Obviously there are exceptions to this as well, in fact more exceptions than the "don't use new" guideline above -- but as you become more experienced both with the syntax and the semantics of the C++ language, you will find that dynamic allocation is needed in fewer scenarios.

Answer 5

The delete keyword should only appear inside the implementation of smart pointer classes. You can either return by value as Cyber suggested, or in cases where that isn't ideal (for example, return by value causes slicing of derived types) you can return std::unique_ptr and store it in a local variable of the same type, and the destructor will automatically clean up the object and its memory when the pointer goes out of scope.

"Doing a class constructor/destructor" for each case would be overkill. Just take advantage of the existing highly reusable smart pointers.

Answer 6

There is no reason to have to chase around memory manually as you are doing. I would just declare your variable on the stack, and return it by value. Then let RAII clean up the memory for you when the variable falls out of scope.

myOptions packageOptions() {
    myOptions options;
    options.myInt = 42;
    options.myString = "hello world";

    return options;
}

int main() {
    myOptions options = packageOptions();

    std::cout << options.myString << std::endl;
    std::cout << options.myInt << std::endl;
}