what's wrong with this code?

Discussion in 'C++' started by Aris, Dec 7, 2005.

  1. Aris

    Aris Guest

    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;

    }
     
    Aris, Dec 7, 2005
    #1
    1. Advertising

  2. Aris wrote:
    > 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
     
    Thomas Tutone, Dec 7, 2005
    #2
    1. Advertising

  3. Aris wrote:
    > 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
     
    Josh Mcfarlane, Dec 7, 2005
    #3
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. walala
    Replies:
    3
    Views:
    2,269
    Ralf Hildebrandt
    Sep 10, 2003
  2. willem oosthuizen

    What is wrong with the following code?

    willem oosthuizen, Oct 10, 2003, in forum: VHDL
    Replies:
    9
    Views:
    1,360
  3. Matthew
    Replies:
    7
    Views:
    875
    Priscilla Walmsley
    Jan 7, 2005
  4. David. E. Goble
    Replies:
    9
    Views:
    508
    David. E. Goble
    Feb 2, 2005
  5. kiran
    Replies:
    12
    Views:
    1,225
    Scott Sauyet
    Dec 7, 2011
Loading...

Share This Page