Help with FIFO QUEUE

This is a discussion on Help with FIFO QUEUE within the C++ Programming forums, part of the General Programming Boards category; Originally Posted by R.Stiltskin No, show me what you did to removeItem(). Edit: and you also have to correct printList() ...

  1. #16
    Registered User
    Join Date
    Feb 2009
    Posts
    72
    Quote Originally Posted by R.Stiltskin View Post
    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.

  2. #17
    Registered User
    Join Date
    Feb 2003
    Posts
    595
    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()

  3. #18
    Registered User
    Join Date
    Feb 2009
    Posts
    72
    Quote Originally Posted by R.Stiltskin View Post
    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?

  4. #19
    Registered User
    Join Date
    Feb 2003
    Posts
    595
    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).

  5. #20
    Registered User
    Join Date
    Feb 2009
    Posts
    72
    Quote Originally Posted by R.Stiltskin View Post
    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
    Last edited by jackfraust; 04-03-2009 at 06:26 AM.

  6. #21
    Registered User
    Join Date
    Feb 2003
    Posts
    595
    As I said before, you still haven't fixed printList(). You need a loop to print every node.
    I said last night:
    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.

  7. #22
    Registered User
    Join Date
    Feb 2009
    Posts
    72
    Quote Originally Posted by R.Stiltskin View Post
    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?
    Last edited by jackfraust; 04-03-2009 at 09:14 AM.

  8. #23
    CSharpener vart's Avatar
    Join Date
    Oct 2006
    Location
    Rishon LeZion, Israel
    Posts
    6,484
    Quote Originally Posted by jackfraust View Post
    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?
    The first 90% of a project takes 90% of the time,
    the last 10% takes the other 90% of the time.

  9. #24
    Registered User
    Join Date
    Feb 2009
    Posts
    72
    Quote Originally Posted by vart View Post
    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.

Page 2 of 2 FirstFirst 12
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Fixing my program
    By Mcwaffle in forum C Programming
    Replies: 5
    Last Post: 11-05-2008, 03:55 AM
  2. Need help with FIFO Queue using Singly Linked Lists
    By astou in forum C++ Programming
    Replies: 6
    Last Post: 03-15-2008, 03:36 PM
  3. help with queues
    By Unregistered in forum C Programming
    Replies: 3
    Last Post: 05-21-2002, 10:09 PM
  4. help with queues
    By Unregistered in forum C Programming
    Replies: 3
    Last Post: 05-21-2002, 12:39 PM
  5. queue help
    By Unregistered in forum C Programming
    Replies: 2
    Last Post: 10-29-2001, 09:38 AM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21