It is legal this approach for create a local variable in C-CodePudding

I'm new to C and try to understand how to create and use a class in C . For this I have the following code:

class MyClass
{ 
    public:
    MyClass()
    {
        _num = 0;
        _name = "";
    }

    MyClass(MyClass* pMyClass)
    {
        _num = pMyClass->_num;
        _name = pMyClass->_name;
    }

    void PrintValues() { std::cout << _name << ":" << _num << std::endl; }
    void SetValues(int number, std::string name)
    {
        _num = number;
        _name = name;
    }

    private:
    int _num;
    std::string _name;
};


int main()
{
    std::vector<MyClass*> myClassArray;
    MyClass myLocalObject = new MyClass();

    for (int i = 0; i < 3; i  )
    {
        myLocalObject.SetValues(i, "test");
        myClassArray.push_back(new MyClass(myLocalObject));
    }

    myClassArray[1]->PrintValues();
    // use myClassArray further   
}

I get a similar example from the internet and try to understand it. My intentions is to populate myClassArray with new class objects. If I compile the code above using VisualStudio 2022 I get no errors, but I'm not sure it doesn't produce memory leaks or if there is a faster and simple approach.

Especially I do not understand the following line: MyClass myLocalObject = new MyClass();

myLocalObject is created on the stack but is initialized with a heap value (because of the new). If new operator is used where should delete must apply?

Thank you for any suggestions!

CodePudding user response：

You have a memory leak at MyClass myLocalObject = new MyClass();, since the dynamically-allocated object is used to converting-construct the new myLocalObject (this was almost but not quite a copy constructor) and then the pointer is lost.

You also didn't show the code using the vector, but if it doesn't delete the pointers inside, you will have more memory leaks.

There's no reason to have an almost-copy-constructor; the compiler has provided you with a better real copy-constructor.

The faster and simpler approach is to recognize that this code doesn't need pointers at all.

class MyClass
{ 
    public:
    MyClass()
        : _num(), _name()   // initialize is better than assignment
    {
        //_num = 0;
        //_name = "";
    }

    // compiler provides a copy constructor taking const MyClass&
    //MyClass(MyClass* pMyClass)
    //{
    //    _num = pMyClass->_num;
    //    _name = pMyClass->_name;
    //}

    void PrintValues() { std::cout << _name << ":" << _num << std::endl; }
    void SetValues(int number, std::string name)
    {
        _num = number;
        _name = name;
    }

private:
    int _num;
    std::string _name;
};


int main()
{
    std::vector<MyClass> myClassArray;  // not a pointer
    MyClass myLocalObject; // = new MyClass();   // why copy a default instance when you can just default initialize?

    for (int i = 0; i < 3; i  )
    {
        myLocalObject.SetValues(i, "test");  // works just as before
        myClassArray.push_back(/*new MyClass*/(myLocalObject)); // don't need a pointer, vector knows how to copy objects
        // also, this was using the "real" copy-constructor, not the conversion from pointer
    }

    myClassArray[1].PrintValues(); // instead of ->
    // use myClassArray further   
}

for cases where a pointer is necessary, for example polymorphism, use a smart pointer:

std::vector<std::unique_ptr<MyClass>> myClassArray;  // smart pointer
myClassArray.push_back(make_unique<MyDerivedClass>(stuff));

std::unique_ptr will automatically free the object when it is removed from the vector (unless you explicitly move it out), avoiding the need to remember to delete.

CodePudding user response：

There are basically 2 ways to instantiate objects of classes.

Dynamic allocation (on heap)

MyClass* myLocalObject = new MyClass(); // dynamically allocates memory and assigns memory address to myLocalObject

Example for your loop:

class MyClass
{
private:
    int _num;
    std::string _name;

public:
    // let's add an additional constuctor having default values
    // that makes it easier later on
    // if parameters are passed, they are used, or the defalt values, if not
    // can call it like MyClass(), MyClass(123), or MyClass(456,"hello")
    // you might want to pass larger data as reference, to avoid copying it
    MyClass(int num=0, std::string name = "some default text")
        : _num(num), _name(name)
    {}
};

std::vector<MyClass*> myClassArray;  // your array of pointers
for (int i = 0; i < 3; i  )
    myClassArray.push_back(new MyClass(i, "test"));

// delete
for (auto& pointerToElement : myClassArray) // get a reference to each element (which is a pointer)
    delete pointerToElement; // delete element (call's destructor if defined)

In that case you must delete myLocalObject; or you get a memory leak.

Instead of dealing with raw pointers, especially when new to C , I recommend to use smart pointers, that deal with memory management for you.

Automatic allocation (on stack when possible)

MyClass myLocalObject = MyClass(); // automatically allocates memory and creates myLocalObject This happens usually on stack (if possible). That's much faster and you don't have to deal with dynamic memory management

Example for your loop:

std::vector<MyClass> myClassArray; // now "containg" the memory of objects itself
for (int i = 0; i < 3; i  )
    {
        myClassArray.emplace_back(i, "test"); // we use emplace_back instead to construct instances of type MyClass directly into the array
    }

// no deletion required here
// destructors of each element will be called (if defined) when myClassArray is deleted automatically when out of scope

There are other ways, like dynamic stack allocation and other black magic, but recommend to focus on "the standard".

In case of dealing with large amounts of data, you might want to use std::vector::reserve. In combination with automatic/stack allocation that helps to speed up a lot by limiting memory allocations to 1 at all instead of 1 per element.

Hope that helps :-)