0

Remove (T &item) is supposed to remove the headPtr and store that "item value" in "item" and return true, otherwise return false if no elements are in the queue. I get a "read access violation error" in this function. Here is the class and function definitions:

class SharedQueue {
public:
    SharedQueue();
    ~SharedQueue();

    //Return true if the queue is empty
    bool IsEmpty() const;

    //Enqueue the next item at the tail of the queue.
    void Add(T value);

    //Dequeue the next queue element and store it in "item" variable.  The function returns false if the queue is empty and no item can be retrieved.
    bool Remove(T &item);
    void Print();
private:
    struct QueueItem {
        T item;
        QueueItem *next;
    };
    //Fill in the The private data members.
    QueueItem *headPtr;
    QueueItem *tailPtr;
};

template <typename T> SharedQueue<T>::SharedQueue() {
    headPtr = NULL;
    tailPtr = NULL;
}

template <typename T>SharedQueue<T>::~SharedQueue() {
    QueueItem *temp;
    while (headPtr != NULL) {
        temp = headPtr;
        headPtr = headPtr->next;
        delete temp;
    }
}

template <typename T> bool SharedQueue<T>::IsEmpty() const {
    if (headPtr == NULL && tailPtr == NULL) {
        return true;
    }
    return false;
}

template <typename T> void SharedQueue<T>::Add(T aValue) {
    if (headPtr == NULL) {
        QueueItem *newItem = new QueueItem;
        newItem->item = aValue;
        tailPtr = headPtr = newItem;
    }
    else {
        QueueItem *newItem = new QueueItem;
        newItem->item = aValue;
        tailPtr->next = newItem;
        tailPtr = tailPtr->next;
    }
}

template <typename T> bool SharedQueue<T>::Remove(T &item) {
    if (headPtr == NULL) {
        return false;
    }
    else {
        QueueItem temp = *headPtr;
        item = headPtr->item;
        delete headPtr;
        headPtr = temp.next;
        return true;
    }
}

template <typename T> void SharedQueue<T>::Print() {
    QueueItem *temp = headPtr;
    while (temp != NULL) {
        std::cout << temp->item << "\n";
        temp = temp->next;
    }
}
Bobby
  • 17
  • 1
  • 8
  • 1
    Why not use `std::queue` instead of a manual implementation? And your class has no copy constructor, so that might screw up your items. See [Rule of Three](https://en.wikipedia.org/wiki/Rule_of_three_%28C++_programming%29). In any case, I would suggest changing the `else` of `Remove()` to this: `QueueItem *temp = headPtr->next; item = headPtr->item; delete headPtr; headPtr = temp;` And BTW, when you remove the last item, don't forget to reset `tail` to NULL, too. – Remy Lebeau Feb 23 '16 at 05:14
  • It's for a class assignment, the instructor gave us the starting code, and we had to fill everything in – Bobby Feb 23 '16 at 05:15
  • 1
    See: http://stackoverflow.com/q/127386/315052 – jxh Feb 23 '16 at 05:17
  • @Bobby: Please show a [Minimal, Complete, and Verifiable example](http://stackoverflow.com/help/mcve) showing how you are actually using this class. – Remy Lebeau Feb 23 '16 at 05:19
  • See http://stackoverflow.com/a/370362/12711 for information about 0xCD filled memory (and other values that fill memory). – Michael Burr Feb 23 '16 at 05:22
  • @RemyLebeau I included the starting code he gave us, and the testing code at the bottom, I haven't done the locks yet – Bobby Feb 23 '16 at 05:27
  • Much of your code included in the question is not relevant to the problem you bring up. Try to cut down your code example to show which code snippets are giving you problems, and then include any code that supports the apparent problem code (because often the problem happens before where your error is thrown)! – Aposhian Feb 23 '16 at 05:40
  • [When and why will an OS initialise memory to 0xCD, 0xDD, etc. on malloc/free/new/delete?](http://stackoverflow.com/q/370195/995714) – phuclv Feb 23 '16 at 05:55
  • Suggestion: For each list operation you need to perform draw the nodes out (on a piece of paper if you have to, bit Visio and OpenOffice's Draw are really helpful) and all of the pre-existing linkages. Then one by one change all of the linkages in all of the nodes needed to insert, delete or whatever. Jot the steps you had to take (and remember one linkage cannot point to two things at once) down and turn those into the logical steps you need to take to make the function work. – user4581301 Feb 23 '16 at 06:18
  • 1
    @Bobby: you have not shown the testing code at all, only the class implementation but none of the code that actually *uses* the class. And btw, since you never set `tail` to NULL after construction, `IsEmpty()` will never return true once at least 1 item has been added even if all items are removed. – Remy Lebeau Feb 23 '16 at 06:23

1 Answers1

3

When you add an item to a non-empty SharedQueue<T>:

    QueueItem *newItem = new QueueItem;
    newItem->item = aValue;
    tailPtr->next = newItem;
    tailPtr = tailPtr->next;

You never set the newItem->next = NULL.

Your private struct QueueItem can (and maybe should) have its own constructor(s)/destructor to help ensure that things like this are set consistently.

Michael Burr
  • 333,147
  • 50
  • 533
  • 760