HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Very simple priority queue

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
prioritysimplequeuevery

Problem

I wrote this code to handle a fixed number of priority levels. I believe performance will depend on the number of priority levels.

Considering only a few levels (3 to 5), is there a more efficient way of doing this? I would also like to know how to improve code quality.

I'm just storing the elements in separate queues based on their priority. And to retrieve, looking for the first queue with element_count > 0 in ascending or descending order depending on the function being called.

For the queues I'm using the code I posted before: Queue implementation in C

priority_queue.h

#ifndef PRIORITY_QUEUE_H
#define PRIORITY_QUEUE_H

#include 
#include 
#include "queue.h"

#define PQ_SUCCESS 0
#define PQ_ERROR 1

#define VERY_HIGH_PRIORITY 4
#define HIGH_PRIORITY 3
#define NORMAL_PRIORITY 2
#define LOW_PRIORITY 1
#define VERY_LOW_PRIORITY 0

typedef struct Priority_Queue Priority_Queue;

Priority_Queue *pq_allocate_custom( uint8_t priority_levels,
                                    size_t lines_per_priority_level, 
                                    size_t line_capacity                );
Priority_Queue *pq_allocate(void);
void pq_free(Priority_Queue *pq);
int pq_insert(Priority_Queue *pq, void *content, uint8_t priority);
int pq_insert_with_default_priority(Priority_Queue *pq, void *content);
void *pq_dequeue_highest(Priority_Queue *pq);
void *pq_dequeue_lowest(Priority_Queue *pq);
void pq_shrink_to_fit(Priority_Queue *pq);

#endif


priority_queue.c

```
#include "priority_queue.h"

#define PQ_PRIORITY_LEVELS 5
#define PQ_LINES 4
#define PQ_LINE_CAPACITY 50

//It just keeps separate queues based on priority.
struct Priority_Queue {
Queue **index; //Queue array
size_t element_count;
uint8_t priority_levels;
};

//Internal methods
static void deallocate_all_queues(Priority_Queue *pq)
{
for(uint8_t i = 0; i priority_levels; ++i)
qu_free(pq->index[i]);
}

//Public methods
//Allocate all data structures according to given values
Priority_Que

Solution

Like @Corbin said, there is little that can be done to improve the efficiency of your code. But there are a few things I can do to improve the code quality.

-
You list define a lot of priorities in your header file.

#define VERY_HIGH_PRIORITY 4
#define HIGH_PRIORITY 3
#define NORMAL_PRIORITY 2
#define LOW_PRIORITY 1
#define VERY_LOW_PRIORITY 0


Put them in an enum instead.

typedef enum {VERY_LOW, LOW, NORMAL, HIGH, VERY_HIGH} priority_t;


You will have to change your struct definition though (and perhaps some other definitions in other areas).

struct Priority_Queue
{
    Queue **index; //Queue array
    size_t element_count;
    enum priority_t priority_levels;
};


-
This is an odd line that may need re-writing. You should change the overall queue struct to be defined as queue_t in my opinion

typedef struct Priority_Queue Priority_Queue; // can be confusing
typedef struct queue_t Priority_Queue; // goes from abstract to concrete, readable


-
You have some very long name definition such as pq_insert_with_default_priority. That can be a task to type without an IDE, and annoying if you misspell it. Perhaps you should make the name shorter.

Code Snippets

#define VERY_HIGH_PRIORITY 4
#define HIGH_PRIORITY 3
#define NORMAL_PRIORITY 2
#define LOW_PRIORITY 1
#define VERY_LOW_PRIORITY 0
typedef enum {VERY_LOW, LOW, NORMAL, HIGH, VERY_HIGH} priority_t;
struct Priority_Queue
{
    Queue **index; //Queue array
    size_t element_count;
    enum priority_t priority_levels;
};
typedef struct Priority_Queue Priority_Queue; // can be confusing
typedef struct queue_t Priority_Queue; // goes from abstract to concrete, readable

Context

StackExchange Code Review Q#38605, answer score: 5

Revisions (0)

No revisions yet.