Return to Revisions

Pattern-less function names. For Queue.h, rather see function named obviously make sense together like Queue_Init(), Queue_GetSize() than queueInit(), getQueueSize(), etc.

Comments with # preprocessing may not be portable

 // #endif /* QUEUE_H_INCLUDED */
 /* QUEUE_H_INCLUDED */
 #endif

There is not need for a head and tail. Alternative, only store tail and have the tail point to the head of the list. End of list detected when p->next == tail->next. This makes your head node one filed smaller. Important if code uses lots of queues.

Unclear why code uses int for the queue size type. A signed type is not needed (could use unsigned) and on a system where size_t could be much wider than int, a queue size like size_t is more prudent. Robust code would check for a queue exceed the max value of size in enqueue().

Good use of size_t for memSize. Good error checking for malloc(). Good to have test case. IMO, a commented sample usage in the *.h file is nice. ; the *.h being the public interface to your good code.

Pedantic: Robust would check for memSize==0 in queueInit() as that negates the correctness of the malloc() checks which should be if(newNode == NULL && q->memSize > 0).

A little documentation goes a long way. suggest a line or two of comment preceding each function declaration in Queue.h.

Functions like queuePeek(Queue *q, void *data) that do not alter *q should be declared queuePeek(const Queue *q, void *data). This self documents the unchanging nature of q in the function to users and allows some optimizations a compiler may not otherwise employ. It is a check on the implementation of the function too.

For completeness, suggest q->memSize = 0 in clearQueue().

Change #include order. Put "Queue.h" first as a check that Queue.h does not depend on the 3 <*.h> include files - unless that .h file is coded in Queue.h.

#include "Queue.h" #include <stdio.h> #include <stdlib.h> #include <string.h> // #include "Queue.h"

Indicate failure. If void dequeue(Queue *q, void *data) does not have anything in the queue to copy to data, there is no indication of that here. Perhaps return true/false indicating success. Same for queuePeek().

For debugging, zero filling memory before free() I have found useful. Errant code tends to fail faster with a 0 pointer/data than with its original data still potentially intact. Faster failing code is easier to debug. YMMV.

Opinion: Storing the queue size is of dubious value, unless of course that is the reason for the queue type - one with a quick size report. Alternative, drop the size field and calculated when needed. More often, I have found then need for bool Queue_IsEmpty(const Queue *q) sufficient than needing a quick size and prefer to drop the ever present size field.