C++ Linked List assignment: trouble with insertion and deletion

I am working on a linked list implementation in C++. I am making progress but am having trouble getting the insertion functionality and deletion functionality to work correctly. Below is list object in the C++ header file:

#ifndef linkList_H
#define linkList_h


//
// Create an object to represent a Node in the linked list object
// (For now, the objects to be put in the list will be integers)
//
struct Node
{
    Node() : sentinel(0) {}

    int number;
    Node* next;
    Node* prev;
    Node* sentinel;
};


//
// Create an object to keep track of all parts in the list
//
class List
{
public:

    //
    // Contstructor intializes all member data
    //
    List() : m_listSize(0), m_listHead(0) {}

    //
    // methods to return size of list and list head
    //
    Node*    getListHead() const { return m_listHead; }
    unsigned getListSize() const { return m_listSize; }

    //
    // method for adding and inserting a new node to the linked list, 
    // retrieving and deleting a specified node in the list
    //
    void  addNode(int num);
    void  insertNode(Node* current);
    void  deleteNode(Node* current);

    Node* retrieveNode(unsigned position);

private:

    //
    // member data consists of an unsigned integer representing
    // the list size and a pointer to a Node object representing head
    //
    Node*    m_listHead;
    unsigned m_listSize;
};

#endif

And here is the implementation (.cpp) file:

#include "linkList.h"
#include <iostream>

using namespace std;


//
// Adds a new node to the linked list
//
void List::addNode(int num)
{
    Node *newNode = new Node;
    newNode->number = num;
    newNode->next = m_listHead;
    m_listHead = newNode;
    ++m_listSize;
}


//
// NOTWORKING: Inserts a node which has already been set to front
// of the list
//
void List::insertNode(Node* current)
{
        // check to see if current node already at
        // head of list
    if(current == m_listHead)
        return;

    current->next = m_listHead;

    if(m_listHead != 0)
        m_listHead->prev = current;

    m_listHead = current;
    current->prev = 0;
}


//
// NOTWORKING: Deletes a node from a specified position in linked list
//
void List::deleteNode(Node* current)
{
    current->prev->next = current->next;
    current->next->prev = current->prev;
}


//
// Retrieves a specified node from the list
//
Node* List::retrieveNode(unsigned position)
{
    if(position > (m_listSize-1) || position < 0)
    {
        cout << "Can't access node; out of list bounds";
        cout << endl;
        cout << endl;

        exit(EXIT_FAILURE);
    }

    Node* current = m_listHead;
    unsigned pos = 0;

    while(current != 0 && pos != position)
    {
        current = current->next;
        ++pos;
    }

    return current;
 }

After running a brief test program in the client C++ code, here is the resulting output:

Number of nodes: 10

Elements in each node:
9 8 7 6 5 4 3 2 1 0

Insertion of node 5 at the list head:
4 9 8 7 6 5 4 9 8 7

Deletion of node 5 from the linked list

As you can see, the insertion is not simply moving node 5 to head of list, but is overwriting other nodes beginning at the third position. The pseudo code I tried to 开发者_Go百科implement came from the MIT algorithms book:

LIST-INSERT(L, x)
    next[x] <- head[L]
    if head[L] != NIL
        then prev[head[L]] <- x
    head[L] <- x
    prev[x] <- NIL

Also the deletion implementation is just crashing when the method is called. Not sure why; but here is the corresponding pseudo-code:

LIST-DELET'
    next[prev[x]] <- next[x]
    prev[next[x]] <- prev[x]

To be honest, I am not sure how the previous, next and sentinel pointers are actually working in memory. I know what they should be doing in a practical sense, but looking at the debugger it appears these pointers are not pointing to anything in the case of deletion:

(*current).prev 0xcdcdcdcd {number=??? next=??? prev=??? ...}   Node *
    number          CXX0030: Error: expression cannot be evaluated  
    next            CXX0030: Error: expression cannot be evaluated  
    prev            CXX0030: Error: expression cannot be evaluated  
    sentinel    CXX0030: Error: expression cannot be evaluated  

Any help would be greatly appreciated!!

You have got an error in addNode(). Until you fix that, you can't expect insertNode to work.

Also, I think your design is quite silly. For example a method named "insertNode" should insert a new item at arbitrary position, but your method insertNode does a pretty different thing, so you should rename it. Also addNode should be renamed. Also as glowcoder wrote, why are there so many sentinels? I am affraid your class design is bad as a whole.

The actual error is that you forgot to set prev attribute of the old head. It should point to the new head.

void List::addNode(int num)
{
    Node *newNode = new Node;
    newNode->number = num;
    newNode->next = m_listHead;

    if(m_listHead) m_listHead->prev = newNode;

    m_listHead = newNode;
    ++m_listSize;
}

Similarly, you have got another error in deleteNode(). It doesn't work when deleting last item from list.

void List::deleteNode(Node* current)
{
    m_listSize--;
    if(current == m_listHead) m_listHead = current->next;
    if(current->prev) current->prev->next = current->next;
    if(current->next) current->next->prev = current->prev;
}

Now you can fix your so-called insertNode:

void List::insertNode(Node* current)
{
    int value = current->number;
    deleteNode(current);
    addNode(value);
}

Please note that I wrote everything here without compiling and testing in C++ compiler. Maybe there are some bugs, but still I hope it helps you at least a little bit.

In deleteNode, you are not handling the cases where current->next and/or current->prev is null. Also, you are not updating the list head if current happens to be the head.

You should do something like this:

node* next=current->next;
node* prev=current->prev;

if (next!=null) next->prev=prev;
if (prev!=null) prev->next=next;

if (m_listhead==current) m_list_head=next;

(Warning: I have not actually tested the code above - but I think it illustrates my idea well enough)

I am not sure what exactly your InsertNode method does, so I can't offer any help there.

OK.

1. As @Al Kepp points out, your "add node" is buggy. Look at Al's code and fix that.

2. The "insert" that you are doing does not appear to be a normal list insert. Rather it seems to be a "move to the front" operation.

3. Notwithstanding that, you need to delete the node from its current place in the list before you add it to the beginning of the list.

Update

I think you have misunderstood how insert should work. It should insert a new node, not one that is already in the list.

See below for a bare-bones example.

#include <iostream>

// List Node Object
//
struct Node
{
    Node(int n=0);
    int nData;
    Node* pPrev;
    Node* pNext;
};

Node::Node(int n)
: nData(n)
, pPrev(NULL)
, pNext(NULL)
{
}

//
// List object
//
class CList
{
public:

    //
    // Contstructor 
    //
    CList();

    //
    // methods to inspect list
    //
    Node*    Head() const;
    unsigned Size() const;
    Node*    Get(unsigned nPos) const;
    void     Print(std::ostream &os=std::cout) const;

    //
    // methods to modify list
    //
    void Insert(int nData);
    void Insert(Node *pNew);
    void Delete(unsigned nPos);
    void Delete(Node *pDel);

private:
    //
    // Internal data
    //
    Node*    m_pHead;
    unsigned m_nSize;
};
/////////////////////////////////////////////////////////////////////////////////
CList::CList()
: m_pHead(NULL)
, m_nSize(0)
{
}
Node *CList::Head() const 
{ 
    return m_pHead; 
}
unsigned CList::Size() const 
{ 
    return m_nSize; 
}
void CList::Insert(int nData)
{
    Insert(new Node(nData));
}
void CList::Insert(Node *pNew)
{
    pNew->pNext = m_pHead;
    if (m_pHead)
        m_pHead->pPrev = pNew;
    pNew->pPrev = NULL;
    m_pHead     = pNew;
    ++m_nSize;
}
void CList::Delete(unsigned nPos)
{
    Delete(Get(nPos));
}
void CList::Delete(Node *pDel)
{
    if (pDel == m_pHead)
    {
        // delete first
        m_pHead = pDel->pNext;
        if (m_pHead)
            m_pHead->pPrev = NULL;
    }
    else
    {
        // delete subsequent
        pDel->pPrev->pNext = pDel->pNext;
        if (pDel->pNext)
            pDel->pNext->pPrev = pDel->pPrev;
    }
    delete pDel;
    --m_nSize;
}

Node* CList::Get(unsigned nPos) const
{
    unsigned nCount(0);
    for (Node *p=m_pHead; p; p = p->pNext)
        if (nCount++ == nPos)
            return p;
    throw std::out_of_range("No such node");
}

void CList::Print(std::ostream &os) const
{
    const char szArrow[] = " --> ";

    os << szArrow;
    for (Node *p=m_pHead; p; p = p->pNext)
        os << p->nData << szArrow;
    os << "NIL\n";
}

int main()
{
    CList l;

    l.Print();

    for (int i=0; i<10; i++)
        l.Insert((i+1)*10);

    l.Print();

    l.Delete(3);

    l.Delete(7);

    l.Print();

    try
    {
        l.Delete(33);
    }
    catch(std::exception &e)
    {
        std::cerr << "Failed to delete 33: " << e.what() << '\n';
    }

    l.Print();

    return 0;
}