Skip to main content
deleted 33 characters in body
Source Link
chux
  • 33.7k
  • 2
  • 38
  • 92
  1. Pattern-less function names. For Queue.h, rather see function named obviously make sense together like Queue_Init(), Queue_GetSize() than queueInit(), getQueueSize(), etc.

  2. Comments with # preprocessing may not be portable

     // #endif /* QUEUE_H_INCLUDED */
     /* QUEUE_H_INCLUDED */
     #endif
    
  3. There is notno 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 filedfield smaller. Important if code uses lots of queues.

  4. 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().

  5. 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.

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

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

  8. 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.

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

  10. 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"

  11. 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().

  12. 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.

  13. 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 calculatedcalculate when needed. More often, I have found thenthe need for bool Queue_IsEmpty(const Queue *q) sufficient than needing a quick size and prefer to drop the ever present size field.

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

  2. Comments with # preprocessing may not be portable

     // #endif /* QUEUE_H_INCLUDED */
     /* QUEUE_H_INCLUDED */
     #endif
    
  3. 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.

  4. 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().

  5. 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.

  6. 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).

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

  8. 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.

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

  10. 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"

  11. 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().

  12. 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.

  13. 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.

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

  2. Comments with # preprocessing may not be portable

     // #endif /* QUEUE_H_INCLUDED */
     #endif
    
  3. There is no 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 field smaller. Important if code uses lots of queues.

  4. 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().

  5. 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.

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

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

  8. 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.

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

  10. 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"

  11. 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().

  12. 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.

  13. 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 calculate when needed. More often, I have found the need for bool Queue_IsEmpty(const Queue *q) sufficient than needing a quick size and prefer to drop the ever present size field.

added 384 characters in body
Source Link
chux
  • 33.7k
  • 2
  • 38
  • 92
  1. Pattern-less function names. For Queue.h, rather see function named obviously make sense together like Queue_Init(), Queue_GetSize() than queueInit(), getQueueSize(), etc.

  2. Comments with # preprocessing may not be portable

     // #endif /* QUEUE_H_INCLUDED */
     /* QUEUE_H_INCLUDED */
     #endif
    
  3. 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.

  4. 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().

  5. 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.

  6. 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).

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

  8. 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.

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

  10. 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"

  11. 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().

  12. 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.

  13. 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.

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

  2. Comments with # preprocessing may not be portable

     // #endif /* QUEUE_H_INCLUDED */
     /* QUEUE_H_INCLUDED */
     #endif
    
  3. 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.

  4. 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().

  5. 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.

  6. 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).

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

  8. 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.

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

  10. 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"

  11. 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().

  12. 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.

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

  2. Comments with # preprocessing may not be portable

     // #endif /* QUEUE_H_INCLUDED */
     /* QUEUE_H_INCLUDED */
     #endif
    
  3. 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.

  4. 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().

  5. 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.

  6. 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).

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

  8. 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.

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

  10. 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"

  11. 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().

  12. 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.

  13. 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.

Source Link
chux
  • 33.7k
  • 2
  • 38
  • 92

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

  2. Comments with # preprocessing may not be portable

     // #endif /* QUEUE_H_INCLUDED */
     /* QUEUE_H_INCLUDED */
     #endif
    
  3. 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.

  4. 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().

  5. 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.

  6. 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).

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

  8. 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.

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

  10. 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"

  11. 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().

  12. 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.