4
\$\begingroup\$

I am learning C and I have some trouble with pointers. I decided to create a queue to practice a bit. The program works as intended, however I want to know some good practices and suggestions.

Here is queue.c:

#include <stdlib.h>
#include "bank_sim.h" 
#include "queue.h"

void initiliaze(Queue *queue) {
        queue->head = NULL;
        queue->tail = NULL;
}

void enqueue(Queue *queue, Customer *customer) {
        if (queue->head == NULL) {
                queue->head = customer;
                queue->tail = customer;
                queue->customer_cnt++;
        } else {
                queue->tail->next = customer;
                queue->tail = customer;
                queue->customer_cnt++;
        }
}

Customer *dequeue(Queue *queue) {
        if (queue->head == NULL) {
                return NULL;
        } else {
                Customer *head_customer = queue->head;
                queue->head = queue->head->next;
                head_customer->next = NULL;
                queue->customer_cnt--;
                return head_customer;
        }
}

queue.h:

#ifndef QUEUE_H
#define QUEUE_H
#include "bank_sim.h"

typedef struct {
        Customer *head;
        Customer *tail;
        int customer_cnt;
} Queue;

void initiliaze(Queue *queue);
void enqueue(Queue *queue, Customer *customer);
Customer *dequeue(Queue *queue);

# endif

Finally bank_sim.h:

#ifndef BANK_SIM_H
#define BANK_SIM_H

typedef struct Customer Customer;

struct Customer {
        int id;
        int service_time;
        struct Customer *next;
};

typedef struct {
        Customer *customer;
        int occupied;
} Bank_Teller;

#endif

Comments regarding anything is welcome including whether my structure for Customer and Queue can be improved.

\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

One major thing: Queue consists of node which consist of data. Here, your data is Customer. Data should not be coupled with the Queue i.e. think this way.

   typedef struct Node{
         Node* next; // navigation pointer.
         Customer customer; // data
   }Node;

Considering the current implementation, here are a few points to go with:

  1. Initialize misses to initialize customer_cnt = 0;.

  2. Where are you allocating the Queue instance. Instead, have getQueueNode which allocates memory for the node and initialise that as well. Allocation at one place helps for e.g: easy debugging when malloc returns null due to insufficient memory.

  3. Extract common code outside the if-else block in enqueue().

  4. Put related code at once i.e. in dequeue operations for head_customer could be consecutive.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.