0

In a C++ program I define several data structures and the class DataClass.

Content of DataClass.h :

typedef struct
{
    int   ta;
    long  tb;
} DataTrgType;

typedef std::vector <DataTrgType> TRG_Type;

typedef struct
{
    int  a;
    long b;
    bool c;
    TRG_Type*     TRG;
} DataType;


class DataClass
{
private:
    DataType*        myData;
    std::vector <DataType>      myDatas;
    DataTrgType*                dataTRG;


public:
  DataClass(std::string pLogFile);
  virtual ~DataClass();

    void createData();
    void setA (int a);
    void setB (long b);
    void setC (bool c);
    void addData();

    void createDataTRG();
    void setTA (int ta);
    void setTB (long tb);
    void addDataTRG ();
};

DataClass.cpp :

void DataClass::createData()
{
    if (myData != NULL)
    {
        delete myData;
        myData = NULL;
    }

    myData = new DataType;
}

void DataClass::addData()
{
    if (myData != NULL)
    {
        myDatas.push_back(*myData);
        delete myData;
        myData = NULL;
    }
}


void DataClass::createDataTRG()
{
    if (dataTRG != NULL)
    {
        delete dataTRG;
        dataTRG = NULL;
    }

    dataTRG = new DataTrgType;
}


void DataClass::addDataTRG ()
{
    if (dataTRG != NULL)
    {
        (*(myData->TRG)).push_back(*dataTRG);

        delete dataTRG;
        dataTRG = NULL;
    }
}

In main I run this code:

  DataClass classObj;

  classObj.createData();
  classObj.setA (11);
  classObj.setB (12);
  classObj.setC(false);
        classObj.createDataTRG();
        classObj.setTA (110);
        classObj.setTB (112);
        classObj.adddataTRG ();

        classObj.createDataTRG();
        classObj.setTA (105);
        classObj.setTB (107);
        classObj.adddataTRG ();
    classObj.addData();


  classObj.createData();
  classObj.setA (21);
  classObj.setB (22);
  classObj.setC(false);
        classObj.createdataTRG();
        classObj.setTA (210);
        classObj.setTB (212);
        classObj.adddataTRG ();
  classObj.addData();

In the program I correctly display the content of these data structures with:

typename std::vector <DataType> :: iterator it;
    typename std::vector <DataTrgType> :: iterator itTrg;
    
    std::cout << "myDatas has " << myDatas.size() << " elements" << std::endl;

    for (it = myDatas.begin(); it != myDatas.end(); ++it)
{
  std::cout << "Data.a = " << (*it).a << " Data.b = " << (*it).b << std::endl;

  for (itTrg = (*it).TRG->begin(); itTrg != (*it).TRG->end(); ++itTrg)
  {
    std::cout << "    Trg.ta = " << (*itTrg).ta << " Trg.tb = " << (*itTrg).tb) << std::endl;
   }
  }

In the destructor of the class I want to release the memory.

I have tried with this code:

DataClass::~DataClass()
{
  typename std::vector <DataType> :: iterator it;

  typename std::vector <DataTrgType> :: iterator itTrg;

  for (it = myDatas.begin(); it != myDatas.end(); ++it)
  {
    for (itTrg = (*it).TRG->begin(); itTrg != (*it).TRG->end(); ++itTrg)
    {
      delete (*itTrg);
    }
    delete it;
  }
}

But it does not compile, showing numerous errors in the destructor.

I have done some other test, but it doesn't compile either and I can't find the correct code to free the memory.

for (int i=0; i<myDatas.size(); i++)
  delete (myDatas[i]);

myDatas.clear();

Any help or suggestion is appreciated.

jstechg
  • 123
  • 1
  • 2
  • 10
  • 1
    Normally the compiler should tell you exactly what syntax problem you got. – pptaszni May 03 '22 at 09:18
  • 3
    Don't use owning bare pointers. – eerorika May 03 '22 at 09:19
  • 4
    don't use raw owning pointers. Store the objects instead of the pointers or use smart pointers. If done right no `new` or `delete` should be present in your code. Using `new` and `delete` manually means you have a bug in your code – 463035818_is_not_an_ai May 03 '22 at 09:19
  • Please post a [mcve] including the complete compiler error. – 463035818_is_not_an_ai May 03 '22 at 09:20
  • 1
    @463035818_is_not_a_number _"Using `new` and `delete` manually means you have a bug in your code"_ — I wouldn't be that strict. Sometimes, we cannot avoid them (such as with custom deleters, or when separating storage allocation and object initialization). – Daniel Langr May 03 '22 at 09:24
  • 2
    There's no need to `typedef struct { /* ... */ } DataTrgType;` in C++: just do `struct DataTrgType { /* ... */ };` – paolo May 03 '22 at 09:24
  • 1
    Or when implementing our own containers without wanting to get any smart pointer overhead. I never understood this `new` hatred. – Fareanor May 03 '22 at 09:25
  • @DanielLangr right. I allowed myself to be sloppy ;). `new` and `delete` is of course not "evil" nor should it be banned completely, though it should be confined to the narrowest scope possible, rather than spread out to code that should not deal with manual memory managment – 463035818_is_not_an_ai May 03 '22 at 09:26
  • @DanielLangr Technically, sure. But it's extremely rare to both need to separate allocation and object creation, *and also* take the shortcut of not using user a provided allocator. – eerorika May 03 '22 at 09:30
  • 2
    @Fareanor what overhead does `std::unique_ptr` have? – Caleth May 03 '22 at 09:51
  • Knives are dangerous too, please stop using knives. – knivil May 03 '22 at 10:00
  • @Caleth [Why can a T* be passed in register, but a unique_ptr cannot?](https://stackoverflow.com/q/58339165/2752075). – HolyBlackCat May 03 '22 at 10:15
  • @HolyBlackCat I would note that this overhead applies only in some scenarios. In many other uses cases, a unique pointer may be as efficient as raw pointer. Unique pointers are rarely passed by values to functions (the problem discussed in the linked question). However, the same issue may apply also for returning values, which is more frequent: https://godbolt.org/z/8hbv8W7sE. – Daniel Langr May 03 '22 at 10:21
  • @Caleth `std::unique_ptr` adds an extra level of indirection. Nothing comes for free (it should be obvious). You still can check the assembly generated for the same code with both versions (with and without the use of `std::unique_ptr`). Of course it's safer, but safety has a cost (even if it is very small in this case). If you write a container that owns a pointer inside, I don't see the need of `std::unique_ptr`, the rule of 5 is here for that. – Fareanor May 03 '22 at 11:25
  • @Fareanor if it all gets inlined, which is *plausible* for the `private`s of a type, then its not another level of indirection. Functions that don't straddle translation unit boundaries don't have to have the same calling convention as those that do – Caleth May 03 '22 at 11:28
  • `std_unique_ptr` is still an intermediary between the user and the actual pointer, inlined or not, everytime you want to access the data inside, you'll have that extra indirection, you've no choice, you can't access the data without passing by the unique pointer interface. – Fareanor May 03 '22 at 11:36

0 Answers0