CODEWITHSUNDEEP
c++overloading
Nolan Anderson
Nolan Anderson

Reputation: 558

Differences when overloading operator+

Something peculiar happened when I was overloading operator+

I have a singly-linked list, and I overloaded the operator+ and operator=

Here's my implementation for both (edit: with insertFront and copy constructor):

 AnyList::AnyList(const AnyList& list)
{
count = list.getCount();
first = list.getFirst();
AnyList a;
a.setFirst(first);
a.setCount(count);

Node *current = first;
for (int i = 0; i < count; ++i)
{
    a.insertFront(current->getData());
    current = current ->getLink();
}
}

-

      void AnyList::insertFront(int value)
{
Node *newNode = new Node;
newNode -> setData(value);
newNode -> setLink(first);
first = newNode;
++count;
}

AnyList AnyList::operator+ (const AnyList& list) const
{

Node *current = first;
Node *listCurrent = list.getFirst();

int sumHolder = 0;
AnyList temp;
while(current != NULL)
{
    sumHolder = current ->getData() + listCurrent ->getData();
    temp.insertFront(sumHolder);
    current = current ->getLink();
    listCurrent = listCurrent ->getLink();
}

return temp;
}


AnyList& AnyList::operator=(const AnyList& rightSide)
{

if(&rightSide != this)
{
    Node *travel = rightSide.getFirst();

    first = rightSide.getFirst();

    Node *original = first;

    while (travel != NULL)
    {
        original ->setData(travel ->getData());
        original ->setLink(travel ->getLink());
        travel = travel->getLink();
        original = original ->getLink();
    }
}

return *this;
}

Here is what is in main:

AnyList mylist;
AnyList mylist2;

for (int i = 0; i < 10; ++i)
{
    mylist.insertFront(i);
}
for (int i = 10; i < 20; ++i)
{
    mylist2.insertFront(i);
}

mylist.print();
cout << endl << endl;
mylist2.print();
cout << endl;
AnyList sumList = mylist + mylist2;
sumList.print();
cout << endl;

My output is as follows (which is the output I desired):

    9 8 7 6 5 4 3 2 1 0 

19 18 17 16 15 14 13 12 11 10

10 12 14 16 18 20 22 24 26 28

So, my question is, when I instead write:

    AnyList sumList;
sumList = mylist + mylist2;
sumList.print();

I get a bad-access error when it goes into the print function and attempts to return the data from the function getData()

I'm super unaware of why this is the case, any help would be greatly appreciated!

Thanks!

Upvotes: 0

Views: 87

Answers (4)

Mike Seymour
Mike Seymour

Reputation: 254471

Your assignment operator copies the "first" pointer from the other list, so that both lists end up sharing the same nodes. (The loop in the operator is just a long-winded way of doing nothing, since you're assigning each node's data an pointer to themselves). Presumably, the list has a destructor which deletes its nodes, so you will end up with both lists trying to delete the same nodes; hence the error (or some other undefined behaviour).

If you want the list to be copyable and assignable at all, which is perhaps not a good idea, then you will have to do a "deep" copy, creating new nodes for the assigned-to list. Also, remember to delete the old nodes before reassigning your pointers to them.

Better still, unless you're learning how to implement a linked list, use the containers provided by the standard library.

Upvotes: 0

Jack Aidley
Jack Aidley

Reputation: 20107

Why do you get an error in one case and not the other? Because AnyList sumList = mylist + mylist2; does not call the = operator but instead invokes the copy constructor. Whereas

AnyList sumList;
sumList = mylist + mylist2;

calls the default constructor and then invokes the = operator.

You haven't shown us enough of your code to be sure why it is crashing when you use the operator= but I'd suggest that Luchian Grigore's answer probably has the right of it.

Upvotes: 0

user1773602
user1773602

Reputation:

This code 

AnyList sumList = mylist + mylist2;

calls a copy constructor rather than operator= (see copy initialization)

And this 

sumList = mylist + mylist2;

Indeed calls operator=

Most probably your copy constructor performs a deep copy, which is correct and doesn't lead to a crash. Your operator= does shallow copy and you get a crash as a result

Upvotes: 1

Luchian Grigore
Luchian Grigore

Reputation: 258618

mylist + mylist2 creates a temporary, whose destructor is called at the end of the full expression. Your assignment operator does a shallow copy, so if the memory is freed by the temporary's destructor being called, sumList will now have dangling pointers.

Read up on how to do deep copies.

To test this, you can temporarily remove the destructor. Just to test though - you'll need it if you're managing resources.

Upvotes: 3

Related Questions