Hey everyone,

I've finally gotten my program to build with no errors or warnings and I can run it but it doesn't give all of the correct output. The Instructor provided my class with what the correct output should be and my program gets it partly right just not all of it. Here's the correct output:

Queue properties:
isEmpty = 1
length = 0
size = 10
first element = -1

Queue properties:
isEmpty = 0
length = 5
size = 10
first element = 10

Queue elements:
10
13
32
3
16

Queue properties:
isEmpty = 1
length = 0
size = 10
first element = -1

Queue elements:
The queue is empty!

Queue properties:
isEmpty = 0
length = 10
size = 10
first element = 0

Queue elements:
0
10
20
30
40
50
60
70
80
90

And here's the output that I get from my program:

Queue properties:
isEmpty = 1
length = 0
size = 10
first element = -1

Queue properties:
isEmpty = 0
length = 5
size = 10
first element = 10

Queue elements:
10
13
32
3
16
-1

Queue properties:
isEmpty = 1
length = 0
size = 10
first element = -1

Queue elements:
The queue is empty!

ErrorErrorErrorErrorErrorErrorErrorErrorErrorErrorQueue properties:
isEmpty = 0
length = 10
size = 10
first element = 0

Queue elements:
-1

(I tried to put them side by side for easier comparison but it wouldn't keep the formatting.)

Ok, and here is my code:

Header file:

// Queue.h: This file contains the definition and implementation
// for the queue class.
#include <iostream>
using namespace std;
class Queue
{
public:
Queue(int size);
~Queue();
void clear();
bool isEmpty();
void enqueue(int element);
int dequeue();
int getFirstElement();
int getLength();
int getSize();
private:
int size; // The total number of items the queue can hold
int length; // The current number of items in the queue
int *queue;
};
Queue::Queue(int size)
{
this->size= size;
queue = new int[size];
clear();
}
Queue::~Queue()
{
size = -1;
length = -1;
delete [] queue;
}
void Queue::clear()
{
//
// TODO:
// This method deletes all of the items in the
// queue. It should also adjust the length variable
//
	for (int i=0; i < size; i++)
   {
      queue[i] = -1;
   }

   length = -1;
}
bool Queue::isEmpty()
{
//
// TODO:
// This method checks to see if the queue is empty
// return true if the queue is empty, false otherwise
//
	if (length == -1)
		return true;
	else
		return false;
}
void Queue::enqueue(int element)
{
//
// TODO:
// This method adds the element to the end of
// the queue. You should first check if the new
// current length does not exceed the queue size.
//
	
	if (length + 1 >= size)
		cout << "Error";
	else
		length++;
		queue[length] = element;
}
int Queue::dequeue()
{
//
// TODO:
// This method removes and returns the first element
// in the queue. You should first check that the
// queue is empty. Adjust the length of the queue
// after dequeuing the element.
//
	int element = -1;
	if (length + 1 >= size)
	{
		return -1;
	}
	else
	{
		element = queue[0];
	}

	for (int i = 0; i < size; i++)
	{
		queue[i] = queue[i+1];
	}

	return element;
}
int Queue::getFirstElement()
{
//
// TODO:
// This method should only return the first element in the
// queue. This method should not dequeue the element.
// It should return -1 if the queue is empty.
	if (Queue::isEmpty() == true)
		return -1;
	else
		return queue[0];
}
int Queue::getLength()
{
return (length+1);
}
int Queue::getSize()
{
return size;
}

Driver

// Driver.cpp: This file contains the source code to test your
// queue class implementation.
#include <iostream>
#include "Queue_test.h"
using namespace std;
void printQueueProperties(Queue *queue);
void printQueue(Queue *queue);
int main(int argc, char **argv)
{
Queue *queue = new Queue(10);
printQueueProperties(queue);
// Add some nodes to the queue
queue->enqueue(10);
queue->enqueue(13);
queue->enqueue(32);
queue->enqueue(3);
queue->enqueue(16);
printQueueProperties(queue);
printQueue(queue);
// Clear the queue
queue->clear();
printQueueProperties(queue);
printQueue(queue);
// Add more nodes than the queue can hold
for (int i=0; i < 20; i++)
{
queue->enqueue(i*10);
}
printQueueProperties(queue);
printQueue(queue);
// Destroy the queue
delete queue;
return 0;
}
void printQueueProperties(Queue *queue)
{
cout << "Queue properties:" << endl;
cout << " isEmpty = " << queue->isEmpty() << endl;
cout << " length = " << queue->getLength() << endl;
cout << " size = " << queue->getSize() << endl;
cout << " first element = " << queue->getFirstElement();
cout << endl;
cout << endl;
return;
}
void printQueue(Queue *queue)
{
cout << "Queue elements:" << endl;
if (queue->isEmpty())
{
cout << " The queue is empty!" << endl;
cout << endl;
return;
}
while (queue->isEmpty() == false)
{
int element = queue->dequeue();
cout << " " << element << endl;
if (element == -1)
break;
}
cout << endl;
return;
}

Anyway, I'm really at a loss on what I should do in this code to give the correct output. Does anyone have any suggestions?

Edited 6 Years Ago by BobbieJean: n/a

Add in a set of braces between line 71 and 72 and between 73 and 74. This isn't tha main problem but your enqueue was happening regardless of the length.

Trace through your portion that prints out the queue in the driver. After the queue is cleared and the 10 elements are added, you have size = 10 and length = 10. Therefore, length+1 >= size and so dequeue returns -1. In your printQueue function this is taken as a signal to break.

Try making the changes around that and post back if something else arises.

Comments
Jonsca is always a great help! Awesome poster!

Add in a set of braces between line 71 and 72 and between 73 and 74. This isn't tha main problem but your enqueue was happening regardless of the length.

Trace through your portion that prints out the queue in the driver. After the queue is cleared and the 10 elements are added, you have size = 10 and length = 10. Therefore, length+1 >= size and so dequeue returns -1. In your printQueue function this is taken as a signal to break.

Try making the changes around that and post back if something else arises.

I hope this doesn't come out wrong or picky or something but, based on my teacher's instructions, it seems that he didn't intend for us (his students) to change anything in the driver file. That's what really makes me feel like I did something wrong in the header file. I included the driver file so that you guys could see how the methods in my header file are used in case that made anything clearer. So I included the braces that you mentioned in my header file but it didn't seem to change anything and I'm a bit reluctant to change anything in the driver file because of the instructions. On that one, I'm afraid of messing it up more. Sorry, but do you have any ideas about a different way I could go about fixing my output where I only edit my header file? If not, thanks anyway for trying. I really do appreciate it.

Edited 6 Years Ago by BobbieJean: n/a

I was asking you to trace through the printQueue so you could see that your dequeue method is in error. There's no need to change the driver. The if condition within the dequeue method will always be true for length the same as size so any queue of length 10 will return a -1 regardless of how many members are in it.

I was asking you to trace through the printQueue so you could see that your dequeue method is in error. There's no need to change the driver. The if condition within the dequeue method will always be true for length the same as size so any queue of length 10 will return a -1 regardless of how many members are in it.

Oh! Sorry for misunderstanding.

Ok, I thought that made sense so I went and took out the = in that if statement but that didn't work. It built fine but when I ran it, it seemed to be in an infinite loop, printing line after line of some negative 6 or so digit number. I'm still trying to figure out how to change that method but I feel like I'm just going in circles...

Read over this portion again. Your if statement in the block is not doing any of these steps.

// TODO:
// This method removes and returns the first element
// in the queue. You should first check that the
// queue is empty. Adjust the length of the queue
// after dequeuing the element.
//

How do you check if the queue is empty? Use the isEmpty() method. Only if your queue is empty return -1 so that your printQueue function doesn't print anything.

Read over this portion again. Your if statement in the block is not doing any of these steps.

// TODO:
// This method removes and returns the first element
// in the queue. You should first check that the
// queue is empty. Adjust the length of the queue
// after dequeuing the element.
//

How do you check if the queue is empty? Use the isEmpty() method. Only if your queue is empty return -1 so that your printQueue function doesn't print anything.

I assumed that you meant to do something like this but I guess not because it still won't work.

if (Queue::isEmpty() == true)
		return -1;

I've also tried using that method in the if statement various other ways and I just don't get it. As far as I remember, I've never used a method within an if statement.

Edited 6 Years Ago by BobbieJean: n/a

Read over this portion again. Your if statement in the block is not doing any of these steps.

// TODO:
// This method removes and returns the first element
// in the queue. You should first check that the
// queue is empty. Adjust the length of the queue
// after dequeuing the element.
//

How do you check if the queue is empty? Use the isEmpty() method. Only if your queue is empty return -1 so that your printQueue function doesn't print anything.

When I was having trouble long before this, I found this in one of the discussion board forums for my class, posted by my instructor:

The dequeue() method should return the element in queue[0], and shift the proceeding elements up by 1. To do this you will need to declare a temporary element at the top of the method:

int element = -1;

First, you should check if the queue is full, that is if length+1 is greater or equal to size. If so print an error and return -1. If not set element equal to queue[0].

Then use a for-loop should shift all of the elements up by one. Start a element 0 up to size. If your index is i then it could appear something like this within your loop:

queue = queue[i+1]

After the loop return the element variable.

I thought I followed that to a T and that's why my code is written the way it is at the moment. Since you pointed out the TODO section under the dequeue method I noticed that the TODO says to check that the queue is empty but this post from my instructor says to check to see if it is full. I find that to be pretty confusing. Should I find a way to check whether it is empty AND/OR full? Something like:

if (length + 1 >= size && Queue::isEmpty() == true)

or

if (length + 1 >= size || Queue::isEmpty() == true)

Edited 6 Years Ago by BobbieJean: n/a

Myself, it seems much more feasible that he would want you to check if it were empty. Then you return -1 and the output of the numbers stops in printQueue. Based on the instructors own driver code returning a -1 stops the output process dead in its tracks, so why should the program not output a full queue? I could be missing something but that seems fishy to me.

I modified the dequeue method to this (in psuedocode so I don't take away your fun)

IF QUEUE IS NOT EMPTY 
{
   ELEMENT = QUEUE[0]
   SHIFT ELEMENTS DOWN
   DECREMENT LENGTH
}
RETURN ELEMENT;

You'll need to do the initialization of element and other housekeeping too
My output:

Queue properties:
 isEmpty = 1
 length = 0
 size = 10
 first element = -1

Queue properties:
 isEmpty = 0
 length = 5
 size = 10
 first element = 10

Queue elements:
 10
 13
 32
 3
 16

Queue properties:
 isEmpty = 1
 length = 0
 size = 10
 first element = -1

Queue elements:
 The queue is empty!

Queue properties:
 isEmpty = 0
 length = 10
 size = 10
 first element = 0

Queue elements:
 0
 10
 20
 30
 40
 50
 60
 70
 80
 90

The extraneous -1 from the listing ending in 16 has also disappeared.

Myself, it seems much more feasible that he would want you to check if it were empty. Then you return -1 and the output of the numbers stops in printQueue. Based on the instructors own driver code returning a -1 stops the output process dead in its tracks, so why should the program not output a full queue? I could be missing something but that seems fishy to me.

I modified the dequeue method to this (in psuedocode so I don't take away your fun)

IF QUEUE IS NOT EMPTY 
{
   ELEMENT = QUEUE[0]
   SHIFT ELEMENTS DOWN
   DECREMENT LENGTH
}
RETURN ELEMENT;

You'll need to do the initialization of element and other housekeeping too
My output:

Queue properties:
 isEmpty = 1
 length = 0
 size = 10
 first element = -1

Queue properties:
 isEmpty = 0
 length = 5
 size = 10
 first element = 10

Queue elements:
 10
 13
 32
 3
 16

Queue properties:
 isEmpty = 1
 length = 0
 size = 10
 first element = -1

Queue elements:
 The queue is empty!

Queue properties:
 isEmpty = 0
 length = 10
 size = 10
 first element = 0

Queue elements:
 0
 10
 20
 30
 40
 50
 60
 70
 80
 90

The extraneous -1 from the listing ending in 16 has also disappeared.

"...so I don't take away your fun" HaHa!!! That's funny! I was so frustrated:confused: there for a while that when I read that, I thought, "My what?":icon_smile: But seriously, I know you didn't want to just give me the answer and I like that. I'm usually pretty adamant about not having someone just do some of my work for me because I realize that I don't learn anything that way. I did finally get it right. Thank you very much for your help. It's greatly appreciated.

This question has already been answered. Start a new discussion instead.