3

In this particular problem I am having, I am not sure of the proper way to deal with readonly structs, passed to class constructors as a parameter when I want to store the data in the struct, in the class.

For example, I have a struct which contains some settings

typedef struct s_MyStruct{
    int x;
    int y;
    std::string name;
    //... arbitrarily many properties.  
} MyStruct;

and I have a class which takes an instance of MyStruct as a parameter in the constructor

class MyClass {
public:
    MyClass(MyStruct settings){
        //I would like to store the data in settings in the class
    }
}

The class is instantiated in a function like this:

void theFunction(){

    MyStruct m_settings { //.. initialization logic }

    MyClass *mClass = new MyClass(m_settings);

    //a reference mClass is passed somewhere so we can access it later
}

So I know when "theFunction" leaves scope it will deallocate the m_settings variable. I was thinking I could make a copy of the struct, or perhaps store copies of all of the variables. Or maybe instead or initializing the struct on the stack I just do a heap allocation and clean up the struct in the class destructor. I don't know the proper design for this sort of message passing in C++ I am not used to manual memory management coming from a managed language background. What is the standard way to handle this struct full of settings?

3

Preliminary remark

In C++ a struct is in fact a class with all members being public. So you could define it more simply:

struct MyStruct{
    int x;
    //... etc.  
};   // no need for typedef here !  

Pass parameter by const reference to the constructor ?

This being said, the way you use it in your constructor makes in fact a copy of your struct (into the parameter). If you're worried about the size of the struct when it's passed as argument, you could define your constructor as:

class MyClass {
public:
    MyClass(const MyStruct& settings) {  // pass a const reference
        ...
    }
};  // ending semi-colon please ;-) ! 

Want to keep a local copy of the initialisation parameters ?

If you then want to keep a copy of all these parameters in your constructed object, just use a mem-initializer:

class MyClass {
    MyStruct m_settings;   // local copy of parameters
public:
    MyClass(const MyStruct& settings) : m_settings(settings) {  
        ...
    }
};

Don't take unnecessary risk with manual memory allocation

Of course, you could do as you thought: allocate a MyStruct object on the free store and pass a pointer. It's technically working, but this is not the way to go in C++. You risk memory leaking in case of unexpected errors, or if you do'nt take care of it in the destructor. You'd risk shallow copies when copying the struct if you don't provide a copy constructor and an assignment operator. So you'd need to take care of the rule of 3 whereas in the approach I proposed you you can safely benefit from default copy constructor, copy assignment and destructor.

If you'd nevertheless wanted to go this way and take care of all this, consider at least use of a shared_ptr<MyStruct> instead of a raw MyStruct* pointer.

3

In this case, your settings struct and the object you want to construct with those settings share the same lifetime: the duration of a call to theFunction. So the best option is probably the simplest one: don't even bother giving them separate variables in the first place. In modern C++, that could be as concise as:

MyClass mClass({ /* struct arguments */ });

Not only is this more concise, but it's potentially more efficient. Because the struct is an unnamed temporary, it only lives for the duration of this constructor call, which means at least in theory it can safely be moved into the mClass constructor rather than copied.

Also, you should make your MyClass constructor take the settings struct by reference (const reference if possible). Doing pass by value usually forces a copy, which seems completely unnecessary in this case.


In general, it might not be this simple, and the various questions you add on at the end imply you want slightly more general advice.

I don't know the proper design for this sort of message passing in C++ I am not used to manual memory management coming from a managed language background. What is the standard way to handle this struct full of settings?

The big thing to realize is that in modern C++, you should almost never manage memory manually. In particular, you almost always want to use stack-allocated variables or smart-pointers to heap-allocated variables instead of "raw" pointers, and you almost always want to work with classes that implement RAII (i.e., they have destructors which properly clean up after themselves) instead of relying on "naked" new/delete pairs.

In particular, if you don't need mClass to be a pointer (either raw or smart), then don't make it one.

MyClass mClass; // doesn't get any simpler than this

If you really need it to be a pointer, then use a smart pointer to ensure it gets cleaned up correctly no matter what happens.

// the `auto` is deduced to be `std::unique_ptr<MyClass>`
auto mClass = std::make_unique<MyClass>(MyStruct{ /* struct arguments */ });

Your go-to smart pointer should be std::unique_ptr since that (usually) has zero runtime overhead compared to a raw pointer, it enforces unique ownership by not allowing you to copy it (which is usually what you want), and it can be easily converted to other smart pointers like std::shared_ptr if you need more complicated behavior.

Both of these options guarantee the mClass object will be destroyed and deallocated even in the face of exceptions or early returns. You never have to write an explicit "please destroy this now" anywhere in your code.

I highly recommend reading Effective Modern C++ for more details on these issues.

Or maybe instead or initializing the struct on the stack I just do a heap allocation and clean up the struct in the class destructor.

This is entirely possible, but overly complicated and kinda pointless.

2
  • so assume I passed mClass to some other variable and it needed to stay alive beyond the function, would it then make the most sense to use shared_ptr to create the struct and assign it to a class variable? – tt9 May 17 '16 at 21:08
  • This is the sort of thing that Effective Modern C++ will cover in detail, but if it just needs to "stay alive" then simply returning or assigning the unique_ptr is enough, as those operations will move (not copy!) the unique_ptr, which effectively transfers ownership of the object being pointed to. Normally you only need a shared_ptr if you need two or more pointers to the same object at the same time, and it's impossible to know at compile time which one will live longer. If none of that made any sense, then just go read the book. – Ixrec May 17 '16 at 21:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.