How to correctly implement queue with using Linked list

Question

I am currently working on my homework. In my program I had to use queue, so I wrote a queue with Linked list. But it doesn't seem to like my syntax.

So I have struct

  typedef struct Node{
  pthread_t thread;
  int threadID;
  int securityMode;  // nTS = 1, S = 2, U = 3
  //int cluseterHalf;  //either 1st or 2nd
  struct NODE *next;
  int executionTime;
  int isLast;
}NODE;
typedef NODE *Link;

And this is where I tried to do Enqueue.

void Enqueue(Link node, Link *Queue){
  Link previous = NULL;
  Link current = *Queue;
  while (current->isLast){
    previous = current;
    current = current->next;
  }
  if(previous == NULL){
    node->next = current;
    *Queue = node;
  }
  else{
    previous->next = node;
    node->next = current;
  }
}

I tried to change my code a bit, but I am getting this error.

Cluster.c:162:13: warning: assignment from incompatible pointer type
[enabled by default]
     current = current->next;
             ^
Cluster.c:165:16: warning: assignment from incompatible pointer type [enabled by default]
     node->next = current;
                ^
Cluster.c:169:20: warning: assignment from incompatible pointer type [enabled by default]
     previous->next = node;
                    ^
Cluster.c:170:16: warning: assignment from incompatible pointer type [enabled by default]
     node->next = current;

I have tried to look at some stackoverflow questions similar to mine. 1)Question1

So I made many logical and non-logical attempts. I tried to write node->next = &current since next is a pointer and it will get the address value. But it didn't work :( I also tried to do oposit *(node->next) = current

I finally found the correct option for me, but I am not sure If that's what I wanted. I was thinking that I had to have struct NODE *next but If I change NODE *next to NODE next than I don't get those errors. But I get different one:

Cluster.c:25:15: error: field ‘next’ has incomplete type
   struct NODE next;

Can you please tell me how to fix this problem? Thanks!

this line: 'typedef NODE *Link;' is a bad idea as it produced a typedef of a typedef when typedef'ing a pointer, it is much better coding practice to include some kind of indication in the new typedef name as to what it points to and that it is a pointer. suggest "typedef NODE *pNode' and there is never any valid reason to typedef a struct definition. just use 'struct Node' when ever needed and remove the typedef modifier — user3629249
– user3629249, Commented Feb 20, 2015 at 21:34

Lucky · Accepted Answer · 2015-02-20 21:29:42Z

1

Try changing struct NODE *next; to struct Node *next; in definiton of your structure

EDIT:

Looking more on the code, I think you have some problems at the pointer assignments. For example, I think that Link current = *Queue; will assign only data of Queue, not address, therefore you cannot access the "inside". Same problem could be with previous.

Also, I dont really understand what's the purpose of Link, you could go with just NODE

edited Feb 20, 2015 at 21:29

answered Feb 20, 2015 at 21:19

Lucky

857 bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

GreedyAi Over a year ago

Thanks! changing NODE to Node helped. but can u explain a bit why NODE doesn't work and Node does? Cause I thought they were the same thing.

GreedyAi Over a year ago

I think u are wrong on *Queue. Because Queue is already a pointer (Link), so when I pass Queue as argument I pass the pointer to array of Nodes. So I have to add * to make Link current = to Queue. Without * I get error.

Lucky Over a year ago

Because you define name of the structure, NODE, at the very end of the definiton and meanwhile, inside the structure, you use Node as some sort of alias.

GreedyAi Over a year ago

ooh so by that time NODE doesn't really exist right?

Lucky Over a year ago

Exactly, you could say that until you reach NODE, only Node exists. Then it's replaced.

|

user3629249 · Accepted Answer · 2015-02-20 23:31:01Z

1

the posted code will present lots of problems when being maintained. Also the code contains several cluttered areas that make understanding/debugging unnecessarily difficult. Meaningful variable names also greatly helps. Suggest:

struct Node
{
    pthread_t thread;
    int threadID;
    int securityMode;  // nTS = 1, S = 2, U = 3
    //int cluseterHalf;  //either 1st or 2nd
    struct Node *next;
    int executionTime;
    // notice removal of unneeded field isLast
};


void Enqueue(struct Node *newNode, struct Node **Queue)
{
    struct Node *current = *Queue;

    newNode->next = NULL;

    if(NULL == current)
    { // then handle special case of empty linked list
        *Queue = newNode;
    }

    else
    { // else, some nodes already in linked list
        // loop to end of linked list
        while (NULL != current->next)
        {
            // step to next node in linked list
            current = current->next;
        } // end while

        // add node to end of linked list
        current->next = newNode;
    } // end if
} // end function: Enqueue

answered Feb 20, 2015 at 23:31

user3629249

16.6k1 gold badge18 silver badges17 bronze badges

1 Comment

GreedyAi Over a year ago

Thanks! Can you please explain why I should put struct in paramiters in Enqueue? I mean Why should I have struct Node *newNode instead of Node *newNode?

Collectives™ on Stack Overflow

How to correctly implement queue with using Linked list

2 Answers 2

6 Comments

1 Comment

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

6 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Linked

Related