I have a question about new and delete: Should I use delete for the input parameter or member object, e.g.：

https://github.com/jwbecalm/Head-First-Design-Patterns-in-CPP/blob/main/ch01_Strategy/main.cpp

Should I use delete on the object allocated by new FlyNewWay()?

// change behavior in runtime
mallardDuck->setFlyBehavior(new FlyNewWay());

// create custom MallarDuck on stack.
MallarDuck rocketDuck =  MallarDuck(new FlyWthRocket(), new NewQuack());

and in MallardDuck constructor:

https://github.com/jwbecalm/Head-First-Design-Patterns-in-CPP/blob/main/ch01_Strategy/MallarDuck.cpp

MallarDuck:: MallarDuck(){
    cout << "in MallarDuck:: MallarDuck()" << endl;
    // 为MallarDuck 赋值其特有的行为
    m_flyBehavior = new FlyWithWings();
    m_quackBehavior = new SimpleQuack();
}

SHould I use delete to free m_flyBehavior and m_quackBehavior? but these two are members of object.

I can easily add delete in destructor of MallarDuck.

MallarDuck::~MallarDuck(){
    cout << "in ~MallarDuck()" << endl;
     delete m_flyBehavior;   
     delete m_quackBehavior;
}

but how about the input parameter mentioned in the first code block? or is this way to design and pass value not recommended? do you have some suggestions?

the setFlyBehavior() is as follows, if I add delete fb, it will delete m_flyBehavior also for my understanding because both fb and m_flyBehavior point to the same memory address.

void Duck::setFlyBehavior(FlyBehavior* fb){
    m_flyBehavior = fb;
    cout << "in Duck::setFlyBehavior()" <<endl;
}

after the setFlyBehavior(), both fb and m_flyBehavior point to the same memory address. so in the deconstructor: ~MallarDuck(), if I delete m_flyBehavior, the input parameter fb where it point to will be free too. So, the delete in deconstructor ~MallarDuck() is enough. thx .

CodePudding user response：

Yes whenever you use new keyword to allocate some memory then you must always use delete to free up that memory later when no longer needed. Otherwise you will have a memory leak as in your program. In your case you should use delete inside the destructor in the MallarDuck.cpp .

Other solution would be to use smart pointers like unique_ptr so that you don't have to explicitly free the memory.

unique_ptr`'s syntax would something like:

unique_ptr<FlyWithWings> m_flyBehavior(new FlyWithWings());

In case you are confused about whether there is a memory leak or not, you can just add a cout statement in the destructor of FlyWithWings and SimpleQuack to confirm whether the destructor is called on these objects or not. Then accordingly you can use smart pointers if needed.

Steps to confirm if you have memory leak

1. Add some std::cout<<"destructor ran; statement inside the destructors of FlyWithWings and SimpleQuack.
2. Also, in this case don't use delete explicitly on the data members inside the destructor of MallarDuck.
3. Then if you see that the destructor is not ran on the data members m_flyBehavior and m_quackBehavior this will mean you have a memory leak and you need to use delete explicitly on the appropriate data members in the destructor of MallarDuck.

Answer to your Edit

how about the input parameter mentioned in the first code block?

For the statement mallardDuck->setFlyBehavior(new FlyNewWay()); you can use the following code:

void Duck::setFlyBehavior(FlyBehavior* fb){
    //delete the old memory but note again we should check whether the pointer is valid or not
   //check for a valid pointer
    if (m_flybehavior)
    {
       
        delete m_flybehavior;//free the old memory
    }
    
    m_flyBehavior = fb;//so m_flybehavior points to some new memory

    //no need to use delete now on m_flybehavior because  in the destructor ~MallarDuck() you have delete m_flybehavior
    cout << "in Duck::setFlyBehavior()" <<endl;
}

And for the statement

MallarDuck rocketDuck =  MallarDuck(new FlyWthRocket(), new NewQuack());

as the MallarDuck's destructor have delete for the data members, it will free up that memory. Also this assumes the input parameters are assigned to the data members.

CodePudding user response：

All pointers created with new must be deleted at some point. In C you wouldn't usually give a pointer to function like setFlyBehavior(), because it is not obvious who should delete the pointer. However it seems you are using polymorphism, so you need to use some kind of pointer.

If the function is called like setFlyBehavior(new FlyNewWay()), the class should delete the pointer. But the user might also call it with an existing pointer and delete it later. The pointer would be deleted multiple times.

// Incorrect usage
FlyBehavior* way = new FlyNewWay();
duck1->setFlyBehavior(way);
duck2->setFlyBehavior(way);
delete way;

Your class has no way knowing how the user created the pointer. You shouldn't trust that the class is being used correctly. Instead, take std::unique_ptr<FlyBehavior> as parameter and save that in your class.

Create the arguments with std::make_unique:

mallardDuck->setFlyBehavior(std::make_unique<FlyNewWay>());