CODEWITHSUNDEEP
c++constructorraw-pointerinitialization-order
Mr. Clear
Mr. Clear

Reputation: 652

Assign member before constructor run

I have a wired problem and don't find a way around it. It looks like the following:

class MyClass : public BaseClass {
public:
    MyClass() : BaseClass(initializeResource())
    { }

    ~MyClass() {
        delete m_resource;
    }

private:
    Resource *m_resource;
    string initializeResource() {
        m_resource = (*resource initialization here*);
        return m_resource.name();
    }
}

So initializeResource() runs before the constructor runs and therefore before all members are initialized. This has to be that way as the result of it is given to the base constructor. I also have to save information in it that is not available in the base class.

This works because the memory is allocated first and the raw pointer will not be overwritten in the constructors initialization. But it looks very fishy to me and I wanted to ask if this could still go wrong.

To get ahead of some suggestions: Encapsulation would solve this but I need derivation here because there will be variables that might either be of type BaseClass or MyClass. m_resource is a system resource and I need to store it and delete it. The constructor of BaseClass needs the name of the resource and I cannot know it before I initialized it. I might have several instances of it, so save it static would also be complicated.

Upvotes: 1

Views: 105

Answers (3)

Mr. Clear
Mr. Clear

Reputation: 652

Thanks to user4581301. I made the constructor private and created a static create function that first initializes the resource and then calls the constructor. Also thanks to PaulMcKenzie to pointing out the rule of 5.

class MyClass : public BaseClass {
public:
    ~MyClass() = default;
    MyClass(const MyClass &o) = delete;
    MyClass(const MyClass &&o) = delete;
    MyClass &operator=(const MyClass &o) = delete;
    MyClass &operator=(const MyClass &&o) = delete;

    static std::unique_ptr<MyClass> create() {
        std::unique_ptr<Resource> resource = (*resource initialization here*);
        return std::unique_ptr<MyClass>(new MyClass(std::move(resource)));
    }

private:
    MyClass(std::unique_ptr<Resource> resource) :
        BaseClass{resource.name()},
        m_resource{std::move(resource)}
    { }

    Resource *m_resource;
}

I'm still curious to know if my first solution works according to the standard of if I just misused undefined behavior.

Upvotes: 1

HolyBlackCat
HolyBlackCat

Reputation: 96791

Another option: Move your member variable to a helper base class (probably a private base with a protected member).

Inherit from this helper base first, and your BaseClass second, then it's members will be initialized first.

Upvotes: 1

Jan Schultke
Jan Schultke

Reputation: 39869

You can create a private helper constructor:

private:
    MyClass(Resource* resource)
      : BaseClass(resource->name())
      , m_resource(resource)
    { }

public:
    MyClass() : MyClass(new Resource /* or more complex initialization */)
    { }

With this, the resource initialization still happens first and you don't need to assign m_resource prior to its initialization.

I believe your original code was incorrect because the assignment m_resource = ... would be followed by default-initialization of m_resource by the constructor, which would replace the assigned value with an indeterminate one; not to mention, the assignment would be undefined behavior because there is no object to be modified by = yet.

Upvotes: 5

Related Questions