Return to Answer

1. "The user of the priority queue is expected to define their data type in the data.h header. This is to make the code flexible and not use void*."

Code limits the types available to pointers to some struct. The definition mechanism requires a data.h file - this is awkward. Strongly recommend a simpler direct approach and reconsider using void *.

2. "The user has to provide a sort function." This is reasonable although I would call it a compare function.

3. If one still wants to retain the struct element approach, at least use a like name for the .h file, perhaps element.h.

4. The 16 in initial_size = 16; is arbitrary. IMO, an empty "bag" like this priority queue should use minimal space. Perhaps start with 0 and use initial_size as the initial allocation once some something is added.

5. Consider using const for function calls that do not change the state. It conveys codes intent better and possible allows some optimizations.

    priority_queue_empty(const struct priority_queue_t* pq);
   

6. Good use of static functions.

7. In general, good naming convention of functions.

8. I'd expect a function that returns the count of queue members.

9. By naming, I was surprised that priority_queue_top() removed the top priority element and not just report it. Consider 2 functions, one to report and another to pop it off.

10. "priority_queue.h" should document the interface more as this is what users see. In particular, detail the meaning of the return value of int(*compare)(element1, element2) and how it affects the queue. Negative return bubble up element1 or element2. What happens when return value is 0?

11. Advanced concept: state that the int(*compare)() must be consistent, like the compare function for qsort(). It must report the same sign of the result for the same argument pair each time it is called. qsort specifies it this way

1. "The user of the priority queue is expected to define their data type in the data.h header. This is to make the code flexible and not use void*."

   Code limits the types available to pointers to some struct. The definition mechanism requires a data.h file - this is awkward. Strongly recommend a simpler direct approach and reconsider using void *.

2. "The user has to provide a sort function." This is reasonable although I would call it a compare function.

3. If one still wants to retain the struct element approach, at least use a like name for the .h file, perhaps element.h.

4. The 16 in initial_size = 16; is arbitrary. IMO, an empty "bag" like this priority queue should use minimal space. Perhaps start with 0 and use initial_size as the initial allocation once some something is added.

5. Consider using const for function calls that do not change the state. It conveys codes intent better and possible allows some optimizations.

    priority_queue_empty(const struct priority_queue_t* pq);
   

6. Good use of static functions.

7. In general, good naming convention of functions.

8. I'd expect a function that returns the count of queue members.

9. By naming, I was surprised that priority_queue_top() removed the top priority element and not just report it. Consider 2 functions, one to report and another to pop it off.

10. "priority_queue.h" should document the interface more as this is what users see. In particular, detail the meaning of the return value of int(*compare)(element1, element2) and how it affects the queue. Negative return bubble up element1 or element2. What happens when return value is 0?

11. Advanced concept: state that the int(*compare)() must be consistent, like the compare function for qsort(). It must report the same sign of the result for the same argument pair each time it is called. qsort specifies it this way

1. "The user of the priority queue is expected to define their data type in the data.h header. This is to make the code flexible and not use void*."

Code limits the types available to pointers to some struct. The definition mechanism requires a data.h file - this is awkward. Strongly recommend a simpler direct approach and reconsider using void *.

2. "The user has to provide a sort function." This is reasonable although I would call it a compare function.

3. If one still wants to retain the struct element approach, at least use a like name for the .h file, perhaps element.h.

4. The 16 in initial_size = 16; is arbitrary. IMO, an empty "bag" like this priority queue should use minimal space. Perhaps start with 0 and use initial_size as the initial allocation once some something is added.

5. Consider using const for function calls that do not change the state. It conveys codes intent better and possible allows some optimizations.

    priority_queue_empty(const struct priority_queue_t* pq);
   

6. Good use of static functions.

7. In general, good naming convention of functions.

8. I'd expect a function that returns the count of queue members.

9. By naming, I was surprised that priority_queue_top() removed the top priority element and not just report it. Consider 2 functions, one to report and another to pop it off.

1. "The user of the priority queue is expected to define their data type in the data.h header. This is to make the code flexible and not use void*."

Code limits the types available to pointers to some struct. The definition mechanism requires a data.h file - this is awkward. Strongly recommend a simpler direct approach and reconsider using void *.

2. "The user has to provide a sort function." This is reasonable although I would call it a compare function.

3. If one still wants to retain the struct element approach, at least use a like name for the .h file, perhaps element.h.

4. The 16 in initial_size = 16; is arbitrary. IMO, an empty "bag" like this priority queue should use minimal space. Perhaps start with 0 and use initial_size as the initial allocation once some something is added.

5. Consider using const for function calls that do not change the state. It conveys codes intent better and possible allows some optimizations.

    priority_queue_empty(const struct priority_queue_t* pq);
   

6. Good use of static functions.

7. In general, good naming convention of functions.

8. I'd expect a function that returns the count of queue members.

9. By naming, I was surprised that priority_queue_top() removed the top priority element and not just report it. Consider 2 functions, one to report and another to pop it off.

10. "priority_queue.h" should document the interface more as this is what users see. In particular, detail the meaning of the return value of int(*compare)(element1, element2) and how it affects the queue. Negative return bubble up element1 or element2. What happens when return value is 0?

11. Advanced concept: state that the int(*compare)() must be consistent, like the compare function for qsort(). It must report the same sign of the result for the same argument pair each time it is called. qsort specifies it this way

When the same objects (..., irrespective of their current positions in the array) are passed more than once to the comparison function, the results shall be consistent with one another. That is, for qsort they shall define a total ordering on the array,