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?

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.

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.

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

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.)