I am writing a doubly linked list class, and I have a need to overload operator =. Piece of class

template<class T> class List : public Collection<T> { private: class Node { public: Node *_next; Node *_prev; T _data; }; Node *_head; Node *_tail; public: List() : _head(nullptr), _tail(nullptr) {} List(const T arr[]){ for(const T &n: arr) this->push_back(n); } List(const List & copy): _head(nullptr), _tail(nullptr) { Node *temp = copy._head; while (temp != nullptr) { push_back(temp->_data); temp = temp->_next; } } ~List() { while (_head) { _tail = _head->_next; delete _head; _head = _tail; } } List<T> & operator=(const List & l){ this->~List(); Node *temp = l._head; while (temp != nullptr) { push_back(temp->_data); temp = temp->_next; } return *this; } void push_back(const T &data) { Node *newNode = new Node; newNode->_data = data; newNode->_next = nullptr; if (is_empty()) { newNode->_prev = nullptr; _head = _tail = newNode; } else { newNode->_prev = _tail; _tail->_next = newNode; _tail = newNode; } } }; 

As you can see, the method body is copied from the copy constructor. I did not find a simpler way to replace the list (I can’t call the copy constructor over this object and then return it to it?). If you tell me how to shorten this operator (if possible), I will also be grateful?

  • 2
    What is push_back , where is the declaration of the class itself? Why copy the body of a method from a copy constructor at all? Make two methods - clear and append - VTT
  • The destructor deletes all objects, then you create a list that, after assignment, no longer exists (meaningless) and returns the object, where all pointers indicate non-existent objects (a triple error in one statement). Not to mention that the destructor of such an object will be called in the program ... Of course you cannot! - AR Hovsepyan
  • @VTT oh well? What is push_back? Is this a question for my task or for a method as such? And why - I said. Well, since it is so interesting, I threw all the necessary part of the class - xt1zer
  • @ARHovsepyan Does not delete, but releases memory from under objects. And oddly enough, the code works - xt1zer
  • It will work, but the behavior will be undefined, and the assignment will produce an incorrect result. The concept does not work uniquely. Will not work as expected. And: to free memory from objects (as you say), this means that these objects will not exist, which I wrote about ... - AR Hovsepyan

1 answer 1

Make the function swap , which swaps two lists, or rather, their insides. There are only

 void List::swap(List& L) { Node * tmp = _head; _head = L._head; L._head = tmp; // То же для _tail } 

Then your assignment operator is easily expressed through the copy constructor:

 List<T> & List::operator=(const List & l) { List<T> tmp(l); swap(tmp); return *this; } 

Destructor will call himself :)

  • Cool decision! Would it be easier to leave the List l parameter instead of creating a new list in the body of the method? - xt1zer
  • Of course, it is possible and so, just usually everyone writes an assignment statement from a constant reference, well, I did not break the tradition :) Your option - transferring a copy operation to another place is quite good. - Harry