Creating a queue of queue of custom defined objects (using a custom defined queue template)

Question

I am using the following code to construct a queue of 10 queues, each containing 10 person objects consisting of three member variabless. Every member variable is different from the others. Later on, I cycle through the queues using a pair of nested for loops to print out all the attributes, however when I do so, rather than 100 uniquely different values, I am simply receiving a set of garbage data. I have tried many things to diagnose the problem, but to no avail. Could someone please check out the code below and see if they can spot where I'm going wrong?

Note: the original code is more complex than this, but I have identified the adding of the queues to the 'master queue' as being the source of the problem, so I have simplified the code somewhat to make it easier to focus on the parts that matter. If anyone require any further information, the please let me know.

string UserChoiceStr;
int UserChoiceInt;
bool Valid = false;

queue<book> BookList;
queue<string> WaitingListIndex;
queue<queue<person> > WaitingLists;     

int main()
{
    Initialise();
    MainMenu();
    return 0;
}

void Initialise()
{
    queue<person> PersonQueue;

    for (int count1 = 1; count1 <= 10; count1 ++)
    {           
        for (int count2 = 1; count2 <= 10; count2 ++)
        {               
            person PersonObject(1, "String goes here", 2);
            PersonQueue.Add(PersonObject);
        }
        WaitingLists.Add(PersonQueue);
    }
}

queue.h:

#ifndef QUEUE_H
#define QUEUE_H
using namespace std;
template <class Type>
class queue
{
private:
    Type *Contents;
    int Front, Back;
    int QueueSize;
public:
    queue(int queuesize = 10);
    ~queue();
    bool Empty() const;
    bool Full() const;
    bool Remove(Type&);
    bool Add(Type&);

};
#endif

queuetemplate.h

#ifndef QUEUETEMPLATE_H
#define QUEUETEMPLATE_H
#include "queue.h"
using namespace std;

// Constructor
template <class Type>
queue<Type> :: queue(int queuesize): 
    Front(0), Back(0),
    QueueSize(queuesize),
    Contents(new Type[queuesize + 1])
{}

// Destructor
template <class Type>
queue<Type> :: ~queue()
{
    delete[] Contents;
}

// Tests
template <class Type>
bool queue<Type> :: Empty() const
{
    return (Front == Back) ? true : false;
}

template <class Type>
bool queue<Type> :: Full() const
{
    return ((1 + Back) % (QueueSize + 1) == Front) ? true : false;
}

// Push and pop
template <class Type>
bool queue<Type> :: Remove(Type& FrontElement)
{
    if (Empty())
    {
        return false;
    }
    else
    {
        FrontElement = Contents[Front];
        Front = (Front + 1) % (QueueSize + 1);
        return true;
    }
}

template <class Type>
bool queue<Type> :: Add(Type& NewElement)
{
    if(Full())
    {
        return false;
    }
    else
    {
        Contents[Back] = NewElement;
        Back = (Back + 1) % (QueueSize + 1);
        return true;
    }
}
#endif

queue.h and queuetemplate.h have been included in the question. — , Feb 21 '11 at 01:03
Your `queue` class template is missing a copy constructor and copy assignment operator. If your class is a resource management class (`queue` is because it owns the `Contents` array), you must follow the [rule of three](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three). Failing to follow the rule of three can only end in tears. — James McNellis, Feb 21 '11 at 01:06
On a totally unrelated note, `return condition ? true : false` is exactly the same as `return condition`. — James McNellis, Feb 21 '11 at 01:08
This code is far too long and complicated. You haven't even included `book` and `person`, but I don't see that they're necessary. Simplify the code until a) you see the bug, b) the behavior changes or c) you can't go any further. Then, if you still need help, amend your post. — Beta, Feb 21 '11 at 01:34
I'm still having problems with this issue. I've simplified the code as requested. — , Feb 26 '11 at 04:50

score 3 · Accepted Answer · answered Feb 21 '11 at 02:14

You're going to run into all sorts of trouble having queue of queue because queue doesn't implement its copy and assign semantics properly.

Whenever you copy you will be copying the pointer you allocated and then your temporary will be deleted taking the pointer with it, leaving the copy with a dangling pointer.

That your code does not crash out is rather surprising.

Given that STL is probably banned (so you can't have vector<Type> which would work) even though you put in using namespace std (in a header, which is wrong) you need to implement copy and assign, and you would probably want it to be a "deep" copy thus:

template< typename Type >
queue<Type>::queue( const queue<Type> & other ) :
    Contents( new Type[other.QueueSize + 1 ] ),
    Front( other.Front ),
    Back( other.Back ),
    QueueSize( other.QueueSize )
{
}

template< typename Type >
void queue<Type>::swap( queue<Type> & other ) :
{
   // use std::swap for each element or
   Type * tempContents = other.Contents;
   other.Contents = Contents;
   Contents = tempContents;

   // do the same for all the integral values
}

// not the most efficient implementation but it is safe
template< typename Type >
queue<Type> & operator=( const queue<Type> & other )
{
     queue<Type> temp( other );
     swap( temp );
     return *this;
}

Also check out the [Rule of Three](http://en.wikipedia.org/wiki/Rule_of_three_%28C%2B%2B_programming%29) — user470379, Feb 21 '11 at 02:23

Creating a queue of queue of custom defined objects (using a custom defined queue template)

1 Answers1