Skip to main content
added 771 characters in body
Source Link
forsvarir
  • 11.7k
  • 6
  • 38
  • 71

casting and malloc

Unnecessary casting is generally frowned upon. malloc returns a void*, so you don't need to cast it when you're assigned it.

cache = (NODE *) malloc(sizeof(NODE));

can just be

cache = malloc(sizeof(NODE));

Bug

You've got a bug in your dequeue function. It doesn't update tail, which means that if you empty the queue, then start adding to it again you get errors.

enqueue_node(&Q_head, 1, &Q_tail);
printf("%d\n", dequeue(&Q_head));
enqueue_node(&Q_head, 1, &Q_tail);
printf("%d\n", dequeue(&Q_head));

Results in:

1
queue is empty ! ! !
-1

naming

It's a little odd that your push/enqueue methods have have a _node postfix, but your pop/dequeue methods don't. I'd pick one and stick to it.

heads and tails

I'd rather see the head and tail of the queue wrapped up together in a queue structure. This means that you only have to pass one parameter (the queue) into your enqueue/dequeue. It also means that if you decide to change the way you implement the queue, your clients aren't impacted.

Are Queues Stacks?

As you've implemented it, it's possible to call stack functions on queue nodes and queue functions on stack nodes. Is this really desirable? If not, then making the change above to introduce a QUEUE structure that wraps a QNODE head and QNODE tail would mean that you would at least have some level of type safety and the compiler would warn you if you tried to call dequeue on your stack.

casting and malloc

Unnecessary casting is generally frowned upon. malloc returns a void*, so you don't need to cast it when you're assigned it.

cache = (NODE *) malloc(sizeof(NODE));

can just be

cache = malloc(sizeof(NODE));

Bug

You've got a bug in your dequeue function. It doesn't update tail, which means that if you empty the queue, then start adding to it again you get errors.

enqueue_node(&Q_head, 1, &Q_tail);
printf("%d\n", dequeue(&Q_head));
enqueue_node(&Q_head, 1, &Q_tail);
printf("%d\n", dequeue(&Q_head));

Results in:

1
queue is empty ! ! !
-1

naming

It's a little odd that your push/enqueue methods have have a _node postfix, but your pop/dequeue methods don't.

casting and malloc

Unnecessary casting is generally frowned upon. malloc returns a void*, so you don't need to cast it when you're assigned it.

cache = (NODE *) malloc(sizeof(NODE));

can just be

cache = malloc(sizeof(NODE));

Bug

You've got a bug in your dequeue function. It doesn't update tail, which means that if you empty the queue, then start adding to it again you get errors.

enqueue_node(&Q_head, 1, &Q_tail);
printf("%d\n", dequeue(&Q_head));
enqueue_node(&Q_head, 1, &Q_tail);
printf("%d\n", dequeue(&Q_head));

Results in:

1
queue is empty ! ! !
-1

naming

It's a little odd that your push/enqueue methods have have a _node postfix, but your pop/dequeue methods don't. I'd pick one and stick to it.

heads and tails

I'd rather see the head and tail of the queue wrapped up together in a queue structure. This means that you only have to pass one parameter (the queue) into your enqueue/dequeue. It also means that if you decide to change the way you implement the queue, your clients aren't impacted.

Are Queues Stacks?

As you've implemented it, it's possible to call stack functions on queue nodes and queue functions on stack nodes. Is this really desirable? If not, then making the change above to introduce a QUEUE structure that wraps a QNODE head and QNODE tail would mean that you would at least have some level of type safety and the compiler would warn you if you tried to call dequeue on your stack.

Source Link
forsvarir
  • 11.7k
  • 6
  • 38
  • 71

casting and malloc

Unnecessary casting is generally frowned upon. malloc returns a void*, so you don't need to cast it when you're assigned it.

cache = (NODE *) malloc(sizeof(NODE));

can just be

cache = malloc(sizeof(NODE));

Bug

You've got a bug in your dequeue function. It doesn't update tail, which means that if you empty the queue, then start adding to it again you get errors.

enqueue_node(&Q_head, 1, &Q_tail);
printf("%d\n", dequeue(&Q_head));
enqueue_node(&Q_head, 1, &Q_tail);
printf("%d\n", dequeue(&Q_head));

Results in:

1
queue is empty ! ! !
-1

naming

It's a little odd that your push/enqueue methods have have a _node postfix, but your pop/dequeue methods don't.