Help with FIFO QUEUE

Quote:

---

Originally Posted by R.Stiltskin

No, show me what you did to removeItem().

Edit: and you also have to correct printList() as I mentioned above. Otherwise everything will be messed up after the first call to that method.



No. You're not using dynamic allocation inside the nodes, so all you have to do is delete the node and that will automatically deal with the node's data members.

---

Code:

---

void Queue::removeItem()
{       
       
        if (mpHead == NULL)
        return;
        else
        {
        QueueItem* temp = mpHead;
        mpHead = mpHead->getNext();
        delete temp;}
}

       

//end removeItem function

//Erase List
void Queue::eraseList()
{       
        while (mpHead != NULL)
        removeItem();
        mpHead = mpTail = NULL;
}//end eraseList function

//prints contents of queue
void Queue::printList()const
{       
        QueueItem* temp = mpHead;
        cout << mpHead->getData() <<"\t\t"<< mNodeCounter <<endl;
        temp = mpHead->getNext();}

---

thats why i dont get how the program still fails. everything is initialized.
for the QUeueItem constructor i did

Code:

---

QueueItem::QueueItem(char *pData, int id)
strncpy(mData, pData, 30);
mNodeID = id;
mpNext = NULL;}

---

I tried to use mData =pData instead but i got an error.

There's nothing wrong with your constructor. The problem is still in printList(). You deleted the loop so now you're only printing the first node.

And in the loop you should set
temp = temp->getNext()

Quote:

---

Originally Posted by R.Stiltskin

There's nothing wrong with your constructor. The problem is still in printList(). You deleted the loop so now you're only printing the first node.

And in the loop you should set
temp = temp->getNext()

---

Yea, i got the same output that i got before. Don't know where the error is in the code, i can't find it with debug neither. why aren't the nodeID #s correspond to the item that they were outputted with?

Here's the output I get after making the corrections I suggested to you:

Code:

---

one
4
two
4
three
4
four
4
three
8
four
8
five
8
six
8
seven
8
eight
8
seven
8
eight
8
nine
11
ten
11
eleven
11

---

which is exactly what your main() calls for. The numbers don't correspond to the node labels because you aren't printing nodeIDs, you're printing mNodeCounter which is always the ID of the LAST node added to the queue. If you want each nodeID, change printList() to print temp->getID instead.
Your QueueItem constructor, and Queue::remove(), Queue::eraseList() are all OK. So your printList() must still be bad (unless you changed & messed up something else).

Quote:

---

Originally Posted by R.Stiltskin

Here's the output I get after making the corrections I suggested to you:

Code:

---

one
4
two
4
three
4
four
4
three
8
four
8
five
8
six
8
seven
8
eight
8
seven
8
eight
8
nine
11
ten
11
eleven
11

---

which is exactly what your main() calls for. The numbers don't correspond to the node labels because you aren't printing nodeIDs, you're printing mNodeCounter which is always the ID of the LAST node added to the queue. If you want each nodeID, change printList() to print temp->getID instead.
Your QueueItem constructor, and Queue::remove(), Queue::eraseList() are all OK. So your printList() must still be bad (unless you changed & messed up something else).

---

here's what mine look like:

Code:

---

//member function definitions for class Date
#include <iostream> //allows program to output data on screen
using std::cout;
using std::endl;
using namespace std;

#include <iomanip>
using std::setfill;
using std::setw;

#include <cstring>
using std::string;
using std::strcpy;

#include "Queue.h"  //include definition of class Queue from Queue.h



//constructor
Queue::Queue()
{ 
        mpHead = NULL;
        mpTail = NULL;
        mNodeCounter = 0;
}

void Queue::addItem(char *pData)
{
        //create and initialize new QueueItem dynamically
        QueueItem *pQI = new QueueItem(pData, ++mNodeCounter);

        if ( mpHead == 0 ){ //check for empty queue
                mpHead = mpTail = pQI;}
               
        else{
                // link new item onto tail of list using mpTail pointer
               
                mpTail->setNext(pQI);
                mpTail = pQI;
               
        }

}//end addItem function

//function removeItem
void Queue::removeItem()
{       
       
        if (mpHead == NULL){
                return;}
        else
        {
        QueueItem* temp = mpHead;
        mpHead = mpHead->getNext();
        delete temp;}
}

       

//end removeItem function

//Erase List
void Queue::eraseList()
{       
        while (mpHead != NULL)
        removeItem();
        mpHead = mpTail = NULL;
}//end eraseList function

//prints contents of queue
void Queue::printList()const
{       
        QueueItem* temp = mpHead;  //assign mpHead to temporary QueueItem
        cout << mpHead->getData() <<"\t\t"<< mNodeCounter <<endl;
        temp = temp->getNext();
}//end printList function

//destructor
Queue::~Queue()
{
        eraseList();
}

---

Code:

---

//member function definitions for class QueueItem

#include  <iostream> // allows program to output data
using namespace std;

#include  <cstring>

#include "QueueItem.h" // include definition of class QueueItem from QueueItem.h

//QueueItem constructor
QueueItem::QueueItem(char *pData, int id)
{       
        strncpy_s(mData, pData, 30);
        mNodeID = id;
        mpNext = NULL;
       
}// end constructor

void QueueItem::setNext(QueueItem *pItem)
{
        mpNext = pItem;
}// end setNext function


QueueItem* QueueItem::getNext()
{
        return  mpNext;
}//end getNext function

int QueueItem::getId()
{
        return mNodeID;
}//end getId function

const char* QueueItem::getData()
{
        return mData;
}//end getData function

---

As I said before, you still haven't fixed printList(). You need a loop to print every node.
I said last night:

Quote:

---

There's nothing wrong with your constructor. The problem is still in printList(). You deleted the loop so now you're only printing the first node.

And in the loop you should set
temp = temp->getNext()

---

Compare your original printList() with the new one. I didn't say you should eliminate the loop. I meant that in the loop you should be using temp instead of mpHead.

By the way, there's still something missing in removeItem(). It hasn't caused any errors yet because nothing in your program ever uses mpTail (you set values for it, but you never use those values). The problem is that if there is only 1 node in the list and you delete that node, mpTail is still pointing to that deleted node. You need an "else if" block that also sets mpTail to NULL when the last node is deleted.

Quote:

---

Originally Posted by R.Stiltskin

As I said before, you still haven't fixed printList(). You need a loop to print every node.
I said last night:

Compare your original printList() with the new one. I didn't say you should eliminate the loop. I meant that in the loop you should be using temp instead of mpHead.

By the way, there's still something missing in removeItem(). It hasn't caused any errors yet because nothing in your program ever uses mpTail (you set values for it, but you never use those values). The problem is that if there is only 1 node in the list and you delete that node, mpTail is still pointing to that deleted node. You need an "else if" block that also sets mpTail to NULL when the last node is deleted.

---

here is the loop i did:

Code:

---

        QueueItem* temp = mpHead;  //assign mpHead to temporary QueueItem
        while (temp != NULL){
        cout << temp->getData() <<"\t\t"<< mNodeCounter <<endl;
        temp = temp->getNext();}

---

IT WORKS!!!!!! thank you man. do you know any extra reading material i can get so i can better understand this?

Quote:

---

Originally Posted by jackfraust

here is the loop i did:

Code:

---

Queue::printlist() const
if ( mpHead == NULL)
return;
else {
QueueItem* temp = mpHead;
cout << mpNext->getData() << "\t\t" << mNodeCounter << endl;
temp = temp->getNext
}

---

---

loop should have word

for

or

while

in the code


Which loop you see here?

Quote:

---

Originally Posted by vart

loop should have word

for

or

while

in the code


Which loop you see here?

---

I edited my post, it works now. I what i was doing wrong is i used mpHead for the while loop instead of temp. Since temp is the temporary item, that the item that should be changing instead of mpHead which will get removed once i call removeItem();

thanks guys, i will refer anybody who has problem with C++ to this site.