heyyo
heyyo

Reputation: 139

Removing Duplicates in Linked List C++ (Seg Faulting)

I'm not sure why I'm getting this seg fault. I'm thinking it probably has something to do with the list traversal. I'm guessing at the end, it is still trying to traverse but it is seeing a nullptr.

I've tried putting another condition in there to check if nextNode is a nullptr then stop the traversal, but I couldn't get it to work.

What am I missing?

void LinkedList::removeDuplicates() 
{
    Node* traverse = m_front;
    Node* nextNode = traverse->getNext();
    Node* duplicate = nullptr;

    if (!isEmpty())
    {
         while(traverse != nullptr)
         {
             if (traverse->getValue() == nextNode->getValue())
             {
                std::cout << "Found duplicate\n";
                duplicate = nextNode;
                nextNode = nextNode->getNext();

                delete duplicate;
                duplicate = nullptr;

                traverse->setNext(nextNode);
            }

            traverse = nextNode;
            nextNode = nextNode->getNext();
        }
    }
}

Upvotes: 1

Views: 342

Answers (1)

Rob L
Rob L

Reputation: 2530

Your code assumes that the list is sorted, otherwise it won't work at all.

If the list is empty, then traverse will be null and the initialization of nextNode will crash.

The last line will also crash, because the last time traverse will be null, and nextnode will be null.

I'd just rearrange nextNode and change the while condition:

void LinkedList::removeDuplicates() 
{
    Node* traverse = m_front;
    Node* duplicate = nullptr;

    if (!isEmpty())
    {
        Node* nextNode = traverse->getNext();
        while(nextNode != nullptr)
        {
             if (traverse->getValue() == nextNode->getValue())
             {
                std::cout << "Found duplicate\n";
                duplicate = nextNode;
                nextNode = nextNode->getNext();

                delete duplicate;
                duplicate = nullptr;

                traverse->setNext(nextNode);
                continue;  // Don't advance again, we already skipped an element.
            }

            traverse = nextNode;
            nextNode = nextNode->getNext();
        }
    }
}

Edit: Added continue to avoid problem deleting last element and problem skipping three duplicates in a row.

Upvotes: 1

Related Questions