I am working on the project which using pointer (new ext...) and I don't know how to fix it, I couldn't use the delete syntax because it will break the code literally.

list<Virus*> DoClone() 
{
    list<Virus*> l;

    Dengue *d1 = new Dengue(1), *d2 = new Dengue(1);
    for (int i = 0; i < 4; i  )
        d1->m_protein[i] = m_protein[i];
    d1->m_dna = m_dna;
    d1->m_resistance = m_resistance;
    for (int i = 0; i < 4; i  )
        d2->m_protein[i] = m_protein[i];
    d2->m_dna = m_dna;
    d2->m_resistance = m_resistance;

    l.emplace_back(d1);
    l.emplace_back(d2);
    //delete d1;
    //delete d2;
    return l;
}

void DoDie()
{
    this->m_dna = NULL;
    memset(this->m_protein, 0, 4);
    this->m_resistance = 0;
    delete this->m_dna;
}

CodePudding user response：

Smart pointers to the rescue:

list<std::unique_ptr<Virus>> DoClone() 
{
    list<std::unique_ptr<Virus>> l;

    auto d1 = std::make_unique<Dengue>(1);
    auto d2 = std::make_unique<Dengue>(1);

    for (int i = 0; i < 4; i  )
        d1->m_protein[i] = m_protein[i];

    d1->m_dna = m_dna;
    d1->m_resistance = m_resistance;
    for (int i = 0; i < 4; i  )
        d2->m_protein[i] = m_protein[i];
    d2->m_dna = m_dna;
    d2->m_resistance = m_resistance;

    l.emplace_back(std::move(d1));
    l.emplace_back(std::move(d2));

    return l;
}

void DoDie()
{
    m_dna.reset();
    memset(this->m_protein, 0, 4);
    this->m_resistance = 0;
}

It would be best if you learn to use smart pointers ASAP. Here is good lecture on topic.

Also see: C Core Guidelines

R.20: Use unique_ptr or shared_ptr to represent ownership

Reason

They can prevent resource leaks.

Example

Consider:

void f()
{
    X x;
    X* p1 { new X };              // see also ???
    unique_ptr<X> p2 { new X };   // unique ownership; see also ???
    shared_ptr<X> p3 { new X };   // shared ownership; see also ???
    auto p4 = make_unique<X>();   // unique_ownership, preferable to the explicit use "new"
    auto p5 = make_shared<X>();   // shared ownership, preferable to the explicit use "new"
}

This will leak the object used to initialize p1 (only).

Enforcement

(Simple) Warn if the return value of new or a function call with return value of pointer type is assigned to a raw pointer.

Extra:

small refactor:

std::unique_ptr<Dengue> createDengue(int x)
{
    auto d = std::make_unique<Dengue>(x);
    d->m_protein = m_protein; // someone claims this is an std::array so loop is not needed
    d->m_dna = m_dna;
    d->m_resistance = m_resistance;
    return d;
}

list<std::unique_ptr<Virus>> DoClone() 
{
    list<std::unique_ptr<Virus>> l;
    l.emplace_back(createDengue(1));
    l.emplace_back(createDengue(1));
    return l;
}

void DoDie()
{
    m_dna.reset();
    this->m_protein = {}; // someone claims this is an std::array so  this is fine
    this->m_resistance = 0;
}

CodePudding user response：

list<Virus*> DoClone() 
{
    //Why are you using a list of pointers?
    //Are you handing these out?
    list<Virus*> l; // Replace list<Virus> l or use smart pointers

    Dengue *d1 = new Dengue(1), *d2 = new Dengue(1);
    for (int i = 0; i < 4; i  ){
        d1->m_protein[i] = m_protein[i];
    }
    //if the move semantics of any of them are wrong your code will crash, 
    //but you don't show us what these are.
    d1->m_dna = m_dna;
    d1->m_resistance = m_resistance;
    for (int i = 0; i < 4; i  )
        d2->m_protein[i] = m_protein[i];
    d2->m_dna = m_dna;
    d2->m_resistance = m_resistance;

    l.emplace_back(d1);
    l.emplace_back(d2);

    //You definitely shouldn't be deleting these you jsut gave them to a container to hold!
    //delete d1;
    //delete d2;
    return l;
}

void DoDie(ist<Virus*>& myList)
{
    //You set this to nullptr, but try to delete it in 4 lines time. This looks like a leak
    //Also the fact it was a pointer means you definitely copied it wrong above
    //use std::shared_ptr or std::weak_ptr if you want to share things.
    this->m_dna = NULL;
    memset(this->m_protein, 0, 4);
    
    this->m_resistance = 0;
    delete this->m_dna;

    //Lastly you need to free the memory in your container
    for(auto* item : myList){
     delete item;
    }
    myList.clear();

}