0
class LinkedList
{
public:
    LinkedList() : _head(nullptr) {}
    LinkedList(ListElement *newElement) : _head(newElement) {}
    ~LinkedList() {  };
    LinkedList(const LinkedList& LL);
    LinkedList& operator=(LinkedList byValLinkedList);
private:
    ListElement *_head;
}
LinkedList::LinkedList(const LinkedList & LL)
{
    ListElement *curr = LL._head;

    // If Linked List is empty
    if (isEmpty() && curr != nullptr) {
        _head = new ListElement(curr->getValue());
        curr = curr->getNext();
    }

    ListElement *newNode = nullptr;
    while (curr) {
        newNode = new ListElement(curr->getValue());
        curr = curr->getNext();
    }
}

LinkedList& LinkedList::operator=(LinkedList byValLinkedList)
{

std::swap(_head, byValLinkedList._head);
return *this;
}


int main() {
    using namespace std;
    LinkedList LL1(new ListElement(7));
    //..... some insertions
    LinkedList LL2(new ListElement(5));
    //..... some insertions
    LL1 = LL2;  // What is the order ?
    // ..... do something else
    return 0;
}

When LL1 = LL2 is executed, which one is supposed to be called.

I expect the copy-assignment to happen. But the code was executed in the following order

  1. Copy Constructor
  2. Copy-Assignemnt
  3. Destructor

What am i doing wrong ? and why was the destructor called?

LotsInLife
  • 13
  • 1

3 Answers3

1
 LinkedList& operator=(LinkedList byValLinkedList);

Your copy assignment takes its parameter by value. This means that

 LL1=LL2;

needs to make a copy of LL2, in order to pass it by value. That's what "passing by value" means. Hence, the copy constructor.

To avoid doing copy construction, the assignment operator must take its parameter by reference, instead:

 LinkedList& operator=(const LinkedList &byValLinkedList);

That means, if course, you can't quite implement the assignment operator using std::swap. But that would be a different question...

Briefly, you have two options: either implement two copy constructors, one that takes a const reference and one that does not, with the latter being able to use std::swap. Or, declare _head to be mutable.

Breakpoint
  • 428
  • 6
  • 19
Sam Varshavchik
  • 114,536
  • 5
  • 94
  • 148
  • In copy-and-swap, you don't want to avoid calling the copy constructor. You want both copy operations (copy assignment and copy construction) to be implemented just once, in the copy constructor. – Ben Voigt May 31 '16 at 01:19
1

You're not doing anything wrong, this is exactly how copy-and-swap is supposed to work.

The copy constructor gets called to set up the parameter, which is passed by value. That's great, because otherwise your copy-assignment operator would have to contain the code for making a copy. This way, you can reuse the logic in the copy constructor.

Then, the parameter goes out of scope and gets destroyed at the end of the function. Because of the swap call, the parameter now contains the resources which used to be held by *this. Also very desirable, because the destructor takes care of freeing them -- otherwise you would have to write cleanup code for the copy assignment operator to properly get rid of the data which is being replaced by the assignment.

In addition to code reuse, copy-and-swap gives you exception safety. If you did the copy directly into the left-hand object (*this), then if anything went wrong, you've already lost the old value and cannot leave things unchanged. But using copy-and-swap, the copy constructor does its work first -- if anything goes wrong such as running out of memory, *this keeps its previous value.

There's a very extensive explanation of the copy-and-swap idiom here:

Community
  • 1
  • 1
Ben Voigt
  • 277,958
  • 43
  • 419
  • 720
0

In your assignment operator byVallinkedList is passed by value. That LinkedList object is initialized by using your copy constructor