Thread: Is it okay to call the destructor like this?

  1. #1
    Registered User
    Join Date
    Sep 2018
    Posts
    194

    Is it okay to call the destructor like this?

    Code:
    Linked_list& Linked_list::operator= (const Linked_list& list) {
    
        Linked_list::~Linked_list();
        Node* node = new Node, *temp = list.head->next;
    
        while (temp != nullptr) {
            node = temp;
            temp = temp->next;
        }
        head = list.head;
    }
    I'm making a class for linked list. I want the '=' to work so that all the nodes in the current object gets deleted and the nodes are inherited from the passed object.

    Can I call Linked_list::~Linked_list(); like that? Here's what I have under the destructor:
    Code:
    Linked_list::~Linked_list() {
        Node* node = head;
        Node* previous_node;
        while (node->next != nullptr) {
            previous_node = node;
            node = node->next;
            delete previous_node;
        }
    }
    The class has an integer data member. So would calling the destructor like that also destruct the int?

    I also saw that you can write 'delete this;', does that achieve the same thing as here?

  2. #2
    Registered User
    Join Date
    Sep 2018
    Posts
    194
    Btw here's the new operator= I wrote
    Code:
    Linked_list& Linked_list::operator= (const Linked_list& list) {
    
        if (list_length == list.list_length) {
    
            Node* other_node = list.head;
            Node* own_node = head;
            while (other_node->next != nullptr) {
                *own_node = *other_node;
                other_node = other_node->next;
                own_node = own_node->next;
            }
        }
        else if (list_length > list.list_length) {
    
            Node* node = head, *previous_node = node;
    
            for (size_t i = 0; i < list_length - list.list_length; i++) {
                node = node->next;
                delete previous_node;
                previous_node = node;
            }
            head = node;
            node = list.head;
    
            for (size_t i = 1; i <= list.list_length; i++) {
                *previous_node = *node;
                previous_node = previous_node->next;
                node = node->next;
            }
        }
        else {
    
            Node* own_node = new Node, *other_node = list.head;
            head = own_node;
    
            for (size_t i = 0; i < list.list_length - list_length - 1; i++) {
                own_node = new Node;
                *own_node = *other_node;
                other_node = other_node->next;
            }
    
            for (size_t i = 1; i < list_length; i++) {
                *own_node = *other_node;
                own_node = own_node->next;
                other_node = other_node->next;
            }
        }
    }
    Feel free to look at it and tell me if I have missed something, if you want to.

  3. #3
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    27,078
    Don't do that. Rather, implement the copy assignment operator in terms of the copy (or possibly move) constructor, the destructor, and swap function, e.g.,
    Code:
    Linked_list& Linked_list::operator= (Linked_list other) {
        swap(other);
        return *this;
    }
    swap would be a member swap function that just swaps the members of the current Linked_list object with the members of its argument, e.g., swapping the head nodes. If you prefer, you could directly use std::swap on the members.

    EDIT: your second attempt aims to reuse the nodes that already exist instead of destroying them and then making a copy. That's a reasonable alternative to the copy and swap idiom I demonstrated because it can save on having to allocate memory. However, you should be copying node data, not the nodes themselves, since it doesn't make sense to copy the next pointers.
    Last edited by laserlight; 02-23-2019 at 01:53 PM.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  4. #4
    Registered User
    Join Date
    Sep 2018
    Posts
    194
    How do I implement copy assignment in terms of constructor and destructor??
    std::swap interchanges the values.. I don't want to change the argument linked list.

    And by the way would calling the destructor like I had originall posted also call the destructor of the integer type or just call the method that I've written?

    Btw I forgot to mention that Node has operator= defined as so:
    Code:
        void operator = (const Node& node) {
                data = node.data;
            }
    That's why I wrote *own_node = *other_node in the code

  5. #5
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    27,078
    Quote Originally Posted by Nwb
    How do I implement copy assignment in terms of constructor and destructor??
    std::swap interchanges the values.. I don't want to change the argument linked list.
    What I showed you is how the copy and swap idiom can be implemented. You do need std::swap, or a member swap possibly implemented with the help of std::swap, because the idea is to copy the other object into a temporary (i.e., using the copy constructor), swap the current object with the temporary (hence the current object becomes a copy of the other object), and then invoke the destructor when the temporary object (now with the former content of the current object) is destroyed. So, your idea of invoking the destructor happens, except that it is implicit and is invoked for the copy of the other object, not the current object.

    You don't have to worry about the special case of self-assignment because this works perfectly fine even for self-assignment, but if somehow that is a concern, an alternative would be:
    Code:
    Linked_list& Linked_list::operator=(const Linked_list& other) {
        if (this != &other) {
            Linked_list temp(other);
            swap(temp);
        }
        return *this;
    }
    Since self-assignment should be rare, this is unlikely to be an improvement.

    Quote Originally Posted by Nwb
    And by the way would calling the destructor like I had originall posted also call the destructor of the integer type or just call the method that I've written?
    Refer to the C++ standard:
    Quote Originally Posted by C++11 Clause 12.4 Paragraph 15
    Once a destructor is invoked for an object, the object no longer exists; the behavior is undefined if the destructor is invoked for an object whose lifetime has ended (3.8). [ Example: if the destructor for an automatic object is explicitly invoked, and the block is subsequently left in a manner that would ordinarily invoke implicit destruction of the object, the behavior is undefined. — end example ]
    Basically, you should not explicitly invoke the destructor of any object, except to destroy an object that has been constructed and placed into allocated storage by placement new.

    If you really want, what you could have done is to move the destructor's code into a destroy() member function that the destructor will then call. You can then call destroy() in your copy assignment operator, taking care to check for self assignment first. However, the copy and swap idiom would be a better choice in that case.

    Quote Originally Posted by Nwb
    Btw I forgot to mention that Node has operator= defined as so:
    I don't think that is wise as there might be the expectation that I had, since the notion of a next pointer is well known for the nodes of a linked list. If reusing already allocated memory was a concern, I might have implemented the copy assignment operator like this:
    Code:
    Linked_list& Linked_list::operator=(const Linked_list& other) {
        if (this != &other) {
            Node* node = head;
            Node* other_node = other.head;
            Node* previous = nullptr;
    
            while (node && other_node) {
                node->data = other_node->data;
                previous = node;
                node = node->next;
                other_node = other_node->next;
            }
    
            if (node && !other_node) {
                while (node) {
                    Node* next = node->next;
                    delete node;
                    node = next;
                }
    
                if (previous) {
                    previous->next = nullptr;
                } else {
                    head = nullptr;
                }
            }
            else if (!node && other_node) {
                if (!previous) {
                    node = new Node(other_node->data);
                    head = node;
                    previous = node;
                    other_node = other_node->next;
                }
    
                while (other_node) {
                    node = new Node(other_node->data);
                    previous->next = node;
                    previous = node;
                    other_node = other_node->next;
                }
            }
    
            list_length = other.list_length;
        }
        return *this;
    }
    I'm assuming that the Node constructor takes two arguments: the data and the next pointer, with the next pointer defaulting to nullptr. (Originally, I imagined that it would be more efficient when extending the linked list only to terminate the last node's next pointer with a null pointer since the others would have their next pointers overwritten anyway, but that is actually not exception-safe: if somehow new or the Node constructor throws an exception, the existing last node needs to have a null next pointer so that the destructor can properly destroy the nodes even to the abrupt end.)
    Last edited by laserlight; 02-24-2019 at 04:44 AM.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Call to destructor crashes program
    By Ravenlore in forum C++ Programming
    Replies: 4
    Last Post: 05-01-2011, 07:26 AM
  2. Replies: 1
    Last Post: 06-10-2008, 08:38 PM
  3. Call destructor from memory manager
    By The Wazaa in forum C++ Programming
    Replies: 8
    Last Post: 05-22-2007, 12:10 AM
  4. Replies: 6
    Last Post: 01-17-2007, 12:07 PM
  5. call a destructor
    By kashifk in forum C++ Programming
    Replies: 17
    Last Post: 07-17-2003, 06:06 AM

Tags for this Thread