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;
}
CodePudding user response:
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.