Member Avatar for noxee

Hi I'm trying to have an assignment operator for my linked list class so that when I call it, it replaces the current one with the one I pass in.

i.e testList1 = testList2;

I'm trying to get it so that testList1 will point to testList2.

Here is the code for my linked list class:

// ----------------------------------------------------------------------------------------
// LinkedList.cpp
//
// Author: Warren Knox
// Version: 01
// Last Revised: 30/10/2006
// Description: A linked list for storing the document ids for all the documents scanned 
//                into the database.
//  
//-----------------------------------------------------------------------------------------
//-----------------------------------------------------------------------------------------

#include "LinkedList.h"
#include <stdlib.h>

//-----------------------------------------------------------------------------------------
// Clears the entire list
//-----------------------------------------------------------------------------------------

void LinkedList::Clear()
{
    //Start at the beginning of the list
    Node *start = &m_first;

    //Loop through and delete until list is empty
    while (start->GetNext() != NULL)
        Delete(start);

}

//-----------------------------------------------------------------------------------------
// Deletes the node AFTER the one passed in as a parameter
//-----------------------------------------------------------------------------------------
void LinkedList::Delete(Node *before)
{
    // Point a temporary point at the one to be deleted    
    Node *temp = before->GetNext();
    
    // Take that one out of the loop
    before->SetNext(temp->GetNext());
    
    // Delete the unwanted node (release its memory)
    delete temp;
}

//-----------------------------------------------------------------------------------------
// Deletes the node AFTER the one passed in as a parameter
//-----------------------------------------------------------------------------------------
bool LinkedList::Remove(int datum)
{
    Node *last, *current;
    last = &m_first;
    current = m_first.GetNext();
    bool found = false;

    while (!found && (current != NULL))
    {
        if (datum == current->GetData())
        {
            found = true;
            last->SetNext(current->GetNext());
            delete current;
        }
        else
        {
            found = false;
            last = current;
            current = current->GetNext();
        }
    }

    return found;
}

//-----------------------------------------------------------------------------------------
// Inserts a new node into the list in ascending order
//-----------------------------------------------------------------------------------------
bool LinkedList::Insert(int datum)
{
    //Pointer to a new node
    Node *newNode, *current, *last;
    bool proceed = true;
    current = m_first.GetNext();
    int count = 0;

    //Try to create a new node on the heap
    try
    {
        newNode = new Node;
    }
    catch(...)
    {
        return false;
    }


    //Place the data in the new node
    current = m_first.GetNext();
    last = &m_first;
    proceed = true;
    newNode->SetData(datum);

    while (proceed && (current != NULL))
    {
        if (datum >= current->GetData()) 
        {
            if (datum != current->GetData())
            {
                last = current;
                current = current->GetNext();    
            }
            else
            {
                delete newNode;
                return false;
            }
        }
        else
        {
            proceed = false;
        }
    }
    
    last->SetNext(newNode);
    newNode->SetNext(current);

    return true;
}

//-----------------------------------------------------------------------------------------
// Removes the numbers in the calling list that aren't in the new list
//-----------------------------------------------------------------------------------------
void LinkedList::Joint(const LinkedList &newList)
{
    Node *current, *last, *list2ptr;
    bool isIn = false;

    current = m_first.GetNext();
    last = &m_first;
    list2ptr = newList.m_first.GetNext();

    while (list2ptr != NULL)
    {
        while (!isIn && (current != NULL))
        {
            if (current->GetData() == list2ptr->GetData())
            {
                isIn = true;
                last->SetNext(current->GetNext());
                delete current;
            }
            else
            {
                last = current;
                current = current->GetNext();
                isIn = false;
            }
        }
        list2ptr = list2ptr->GetNext();
        current = m_first.GetNext();
        last = &m_first;
        isIn = false;

    }
}

//-----------------------------------------------------------------------------------------
ostream &operator << (ostream &ostr, LinkedList &list)
{
    //ostr << "The document numbers contained in the list:";
    
    for (Node *ptr = &(list.m_first); ptr->GetNext() != NULL; ptr = ptr->GetNext())
    {
        ostr << " 0" << ptr->GetNext()->GetData();
    }
    
    return ostr;
}

//-----------------------------------------------------------------------------------------

Node *LinkedList::Find(int datum)
{
        for (Node *ptr = &m_first; ptr->GetNext() != NULL; ptr = ptr->GetNext())
        {
            if (ptr->GetNext()->GetData() == datum)
                return ptr;
            else
                return &m_first;
        }
}

//-----------------------------------------------------------------------------------------

LinkedList &LinkedList::operator = (const LinkedList &list)
{
    *this = &list;

    return *this;
}
  • 1 Contributor
  • 1 Reply
  • 83 Views
  • 6 Hours Discussion Span
  • Latest Post Latest Post by noxee

Dani AI

Generated

The code in the first post shows a very common mistake: the assignment operator does not perform a copy of the list nodes. The posted operator calls *this = &list;, which does not make the left-hand list become a copy of the right-hand list — it attempts to assign an address rather than copying elements. As later mentioned, the problem was resolved, but for future readers the correct approaches are clearer when explained.

Follow the Rule of Three: implement a destructor, copy constructor, and copy assignment. The simplest, safe idiom is copy-and-swap: implement a copy constructor that clones nodes, implement a swap that exchanges internal pointers/size, then implement operator= by pass-by-value and swap. That handles self-assignment and gives strong exception safety:

void swap(LinkedList &a, LinkedList &b) noexcept {
    using std::swap;
    swap(a.head, b.head);
    swap(a.size, b.size);
}

LinkedList &LinkedList::operator=(LinkedList other) {
    swap(*this, other);
    return *this;
}

Additional tips: test with memory-checkers (Valgrind) to catch leaks; ensure the destructor frees every node; make non-modifying methods const; if aliasing (shared ownership) was intended, use std::shared_ptr instead of raw pointers. For background on the Rule of Three and safe assignment techniques, see the C++ FAQ on special member functions (Rule of Three) and std::swap usage (std::swap).

Member Avatar for noxee

Again disregard what i asked for above, i've worked out something that works for me

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.