Enqueue successful, but Queue prints nothing?

Question

I am trying to enqueue data into a queue.

The enqueue appears to have been successful because the current Node's Thing pointer is not null. Indeed, its Thing id is 7 as expected. However, something is wrong with ThingQueue's print function because the exact same Node is identified as a nullptr. This is despite the fact that both the enqueue and print functions iterate through the queue in the same way. Can anybody explain the discrepancy?

Here is a minimal reproducible example of my problem:

#include <iostream>

using namespace std;

struct Thing
{
    int id;
    
    Thing()
    {
        id = 7;
    }
    
    void print()
    {
        cout << "Thing " << to_string(id) << " ";
    }
    
};

class ThingQueue
{
    
public:
    
    ThingQueue()
    {
        front = nullptr;
    }
    
    void enqueue(const Thing &thing)
    {
        cout << "Attempting to enqueue..." << endl;
        Node *curr = front;
        while (curr != nullptr) curr = curr->next;
        curr = new Node;
        curr->next = nullptr;
        curr->thing = (Thing*) &thing;
        cout << "Is curr->thing nullptr? 1 for true, 0 for false: ";
        cout << to_string(curr->thing == nullptr) << endl;
        cout << "curr->thing->id = " << to_string(curr->thing->id) << endl;
    }
    
    void print()
    {
        cout << "Attempting to print..." << endl;
        Node *curr = front;
        cout << "Is curr nullptr? 1 for true, 0 for false: ";
        cout << to_string(curr == nullptr) << endl;
        while (curr != nullptr) {
            curr->thing->print();
            curr = curr->next;
        }

    }

private:
    
    struct Node
    {
        Node *next;
        Thing *thing;
    };
    
    Node *front;

};

int main() {
    Thing *pThing = new Thing();
    ThingQueue *pThingQueue = new ThingQueue();
    pThingQueue->enqueue(*pThing);
    pThingQueue->print();
    std::cout << std::endl;

    return 0;
}

Output:

Attempting to enqueue...
Is curr->thing nullptr? 1 for true, 0 for false: 0
curr->thing->id = 7
Attempting to print...
Is curr nullptr? 1 for true, 0 for false: 1

SOLUTION:

Thanks for all the tips and feedback. After working out the remaining bugs and feedback, I have a minimum reproducible example that compiles, runs and produces the expected output! (Not worrying about memory management for this example)

#include <iostream>

using namespace std;

struct Thing
{
    int id;
    
    Thing()
    {
        id = 7;
    }
    
    void print()
    {
        cout << "Thing " << to_string(id) << " ";
    }
    
};

class ThingQueue
{
    
public:
    
    ThingQueue()
    {
        front = nullptr;
    }
    
    void enqueue(const Thing &thing)
    {
        if (front == nullptr)
        {
             // queue is empty, set front to new node
             front = new Node;
             front->next = nullptr;
             front->thing = thing;
        }
        else
        {
             // queue is not empty, find last node
             Node* curr = front;
             while (curr->next != nullptr)
                  curr = curr->next;
             // curr now points to the last node, set curr->next to new node
             curr->next = new Node;
             curr->next->next = nullptr;
             curr->next->thing = thing;
        }
    }

    
    void print()
    {
        Node *curr = front;
        while (curr != nullptr) {
            curr->thing.print();
            curr = curr->next;
        }

    }

private:
    
    struct Node
    {
        Node *next;
        Thing thing;
    };
    
    Node *front;

};

class Stuff
{
    
};

int main() {
    Thing thing = Thing();
    Thing thing2 = Thing();
    Thing thing3 = Thing();
    ThingQueue tq = ThingQueue();
    tq.enqueue(thing);
    tq.enqueue(thing2);
    tq.enqueue(thing3);
    tq.print();
    return 0;
}

`Node *curr = front;` makes a variable named `curr` that stores the same value as `front`. `curr` and `front` start with the same address, but they are different variables. you can do anything you want to `curr` without changing `front`, ergo you never actually set `front`. I usually [use a trick like this](https://stackoverflow.com/a/59779376/4581301) to get around the problem. — user4581301, Jun 19 '22 at 19:52
The first few lines of your `enqueue` function involve a bunch of traversal that doesn't really do anything. At the time you do `curr = new Node;`, `curr` is `nullptr` and has no connection to anything in the list. It sounds like you actually want to check `curr ->next` but with a special case for what happens if your queue is empty and `curr->next` doesn't mean anything to begin with. — Nathan Pierson, Jun 19 '22 at 20:42
Please don't add "solved" to the title, and don't add the solution to the question. Instead post an "answer" below with the solution. Accepting one of the answers by pressing the green checkmark automatically marks your question as solved. — HolyBlackCat, Jun 19 '22 at 21:09
@HolyBlackCat But I want to give someone else, not myself, credit for answering my question, and I do want to post my solution for the sake of others who may experience similar difficulty. — K Man, Jun 19 '22 at 21:12
Then you can post an answer, but leave the checkmark on the other person's answer. — HolyBlackCat, Jun 19 '22 at 21:13

john · Accepted Answer · 2022-06-19T20:07:42.947

Look at enqueue, it creates a new Node and sets curr to point at it. But curr is a variable in the enqueue function, it has nothing to do with your queue, as soon as the enqueue function is exited the curr variable is lost.

You also have another problem. For some reason you have made thing a pointer. This means you end up with pointers to variables which have been destroyed. Change thing to be a non-pointer.

Here's how enqueue should look.

void enqueue(const Thing &thing)
{
    if (front == nullptr)
    {
         // queue is empty, set front to new node
         front = new Node;
         front->next = nullptr;
         front->thing = thing;
    }
    else
    {
         // queue is not empty, find last node
         Node* curr = front;
         while (curr->next != null)
              curr = curr->next;
         // curr now points to the last node, set curr->next to new node
         curr->next = new Node;
         curr->next->next = nullptr;
         curr->next->thing = thing;
    }
}

I can see you are suffering from the pointers everywhere syndrome that beginners sometimes have.

Here's main rewritten without all the unnecessary pointers

int main() {
    Thing thing;
    ThingQueue thingQueue;
    thingQueue.enqueue(thing);
    thingQueue.print();
    std::cout << std::endl;
    return 0;
}

The only pointers you need in this code are the next field in your node class, the front member in your queue class, and the curr variable.

Thank you John! After working out the remaining bugs and errors, I got my program to print "Thing 7" three times in a row. (Not worrying about memory management for this example) — K Man, Jun 19 '22 at 21:04

Enqueue successful, but Queue prints nothing?

1 Answers1