0

I'm learning data structures in C++;

I've written a destructor for a linked list as follows:

    ~List(){
        Node* temporary = new Node;
        Node* node = head;
        while(node != nullptr){
            temporary = node;
            node = node->next;
            delete temporary;
        }
    }

But then I realized that I can do:

        ~List(){
        Node* node = head;
        while(node != nullptr){
            node = std::move(node->next);
        }
    }

Avoiding creating a temporary object, I tried it and worked fine, but I don't know if it is okay, I didn't find a destructor like this in any other place.

Eric Cardozo
  • 23
  • 5
  • 2
    `std::move` on a pointer has no effect whatsoever. It is clear you misunderstand something about the object model in C++, but I'm not sure what that misunderstanding is. Can you clarify why you think the move would substitute the `delete`? In the second example, the node are leaked. It is also odd to `new` an object in a destructor. – François Andrieux Nov 22 '22 at 14:27
  • 1
    I think you are misunderstanding how objects in C++ work. This is not Java, I would recommend a [good C++ book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). `std::move` does not call `delete`. – Quimby Nov 22 '22 at 14:28
  • If you are learning data structures, the first thing to learn (IMHO) is that a "linked list" is just about the *worst* data structure you can use. But if you want to insist on terrible, then at least don't code it yourself, use `std::list`. – Jesper Juhl Nov 22 '22 at 14:29
  • @François Andrieux Doesn´t std::move delete the pointer after the the call? – Eric Cardozo Nov 22 '22 at 14:30
  • You've managed to introduce - and leak - a dynamic allocation in the destructor. Definitely second the suggestion to read a good book. – Useless Nov 22 '22 at 14:30
  • 1
    `Node* temporary = new Node;` So in order to demolish a row of houses, you go and build a new house first? I don't think a demolition business that does this will survive for long. – n. m. could be an AI Nov 22 '22 at 14:30
  • @EricCardozo No, it does not. Pointers are a trivial fundamental type and moving them is the same as copying them. A pointer just a hold a value which says "which object". It is not the object it points to. You may be familiar with high level languages like Java or C# where variables are typically references to an instance which manage lifetime, but that isn't how pointers work in C++. – François Andrieux Nov 22 '22 at 14:31
  • "Doesn´t std::move delete the pointer after the the call?" - No. `std::move` is nothing but a cast to rvalue reference. It doesn't do anything on its own. – Jesper Juhl Nov 22 '22 at 14:31
  • moving _can_ be destructive, but that's only if you're using owning smart pointers or other non-trivial types (and it happens when you _use_ the rvalue reference in a move ctor or assignment, not when you do the cast) – Useless Nov 22 '22 at 14:32
  • @Jesper Juhl So everyone should drop ther CS majors and give up programming cause everything is made up alredy, I should look for a job selling cars instead. Why bother to learn c++? If I can do it easier in python? – Eric Cardozo Nov 22 '22 at 14:32
  • @EricCardozo Do what you please. But I'd still advice against linked lists. Besides, university doesn't really produce that great real-life programmers - you always have to re-train them. – Jesper Juhl Nov 22 '22 at 14:34
  • There's no harm in rewriting existing containers for learning IMO, but so far the lesson is just that you don't understand pointers yet. – Useless Nov 22 '22 at 14:35
  • fwiw with `std::unique_ptr` your code would work. – apple apple Nov 22 '22 at 14:50
  • *"I realized that I can do: [code] Avoiding creating a temporary object,"* -- an easier way to avoid creating a "temporary" object is to not create it. (If you remove the `= new Node` from your first code block, you avoid creating the unnecessary object without impacting anything else.) – JaMiT Nov 22 '22 at 15:08

2 Answers2

1

This code snippet

~List(){
    Node* temporary = new Node;
    Node* node = head;
    while(node != nullptr){
        temporary = node;
        node = node->next;
        delete temporary;
    }
}

produces a memory leak due to this unneeded memory allocation

    Node* temporary = new Node;

In this code snippet

    ~List(){
    Node* node = head;
    while(node != nullptr){
        node = std::move(node->next);
    }
}

neither memory is freed. Only the pointer node is reassigned until it will be equal to a null pointer. So there are again numerous memory leaks.

If you do not want to use an intermediate variable then the destructor can be written for example the following way

    #include <functional>

    //...

    ~List(){
        while ( head ) delete std::exchange( head, head->next );
    }
}
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
1

std::move doesn't do anything by it's own, it only cast something to rvalue.

How the rvalue is used is determined by the function that accept it, and assignment of raw pointer does nothing different than copy in that case.

But for example, if you're using std::unique_ptr, the operator=(unique_ptr&&) would delete the original data after assignment*.

so if you're using something like

#include <memory>

struct Node{
   std::unique_ptr<Node> next;
   // possibly move List destructor here
   // i.e. destruct a Node would safely remove all sub-node non-recursively
};

struct List{
   std::unique_ptr<Node> head;
   // write destructor to prevent deep recursion
   ~List(){
      while(head) head = std::move(head->next); // current head is deleted after assignment
   }
};

then it would work

*btw, self-assignment is safe because it's actually effectively reset(r.release())

apple apple
  • 10,292
  • 2
  • 16
  • 36