what's wrong with this code?

[Aris]

Hello!
Im trying to implement a queue
using a linked list.
I've made that code
and I expected my Degueue()
function to return the value of the
key of the node I constructed.
But It does not.
In fact it returns "Empty Queue"
what am I doing wrong?


#include <stdlib.h>
#include <stdio.h>

typedef struct Queue * link;
struct Queue
{
link head;
link tail;
link next;
int key;
};

void Enqueue(link Head,link Tail,int data)
{
link ptr=(struct Queue*)malloc(sizeof(struct Queue));
ptr->key=data;
if(Head==0) Head= ptr; else Tail->next=ptr;
Tail=ptr;
}

int Dequeue(link Head,int data)
{
link temp;
if(Head!=0){
data=Head->key;
temp=Head;
Head=Head->next;
}

else
{
printf("Empty Queue\n"); return 0;
}

free(temp);
return data;
}




int main()
{
link head=0;
link tail=0;

Enqueue(head,tail,1);

int x=Dequeue(head,x);
printf("%d\n",x);



return 0;

}

 

[Thomas Tutone]

Hello!
Im trying to implement a queue
using a linked list.
I've made that code
and I expected my Degueue()
function to return the value of the
key of the node I constructed.
But It does not.
In fact it returns "Empty Queue"
what am I doing wrong?

Several things. First, you've written C code - there's nothing about
this that makes it C++. That suggests you may get better help asking
in a C newsgroup, even though your program is valid C++ as well.

Second, you're reinventing the wheel, since C++ already includes linked
lists, stacks, queues, etc. The C++ standard library implementation
probably is more efficient than your implementation, but it has a much
more important advantage - it doesn't have the bugs your code has.

But I'll give you a couple hints - see below:

#include <stdlib.h>
#include <stdio.h>

typedef struct Queue * link;
struct Queue
{
link head;
link tail;
link next;
int key;
};

void Enqueue(link Head,link Tail,int data)
{
link ptr=(struct Queue*)malloc(sizeof(struct Queue));
ptr->key=data;

Here, you are changing the local variables Head and Tail - and those
disappear as soon as this function returns.

if(Head==0) Head= ptr; else Tail->next=ptr;
Tail=ptr;
}

Poof - they're gone!

int Dequeue(link Head,int data)
{
link temp;
if(Head!=0){
data=Head->key;
temp=Head;

Here again you change the local variable Head - so at the end of the
function, Head disappears.

Head=Head->next;
}

else
{
printf("Empty Queue\n"); return 0;
}

free(temp);
return data;
}

int main()
{
link head=0;
link tail=0;

Enqueue(head,tail,1);

int x=Dequeue(head,x);
printf("%d\n",x);



return 0;

}

There are other problems with your code too. Presumably you need
variables for head and tail. But why not just use std::deque or
std::stack?

Best regards,

Tom

 

[Josh Mcfarlane]

void Enqueue(link Head,link Tail,int data)

if you want to assign values to the variables passed as parameters, you
need to use a reference to them. Currently the function creates a copy
of the variables passed and uses them.

Instead of link, you should use link& when you want a reference to a
variable passed to the function.

int Dequeue(link Head,int data)

What's the point of the data parameter? Why not just use the return
type int and a parameter to the head of the list?

Josh McFarlane