c programming, queue and modules problems

Question

I'm creating a simple queue with simple operations. I use an ADT item to contain the informations, in this case only an int value. Then I use this item in the nodes. These are the files:

item.h

typedef struct c_item *item;

item newItem(int x);
int eq(item x, item y);
item input_item();
void output_item(item x);
char* getx(item x);
item cloneItem(item x);

item.c

#include <memory.h>
#include <stdio.h>
#include <stdlib.h>
#include "item.h"

struct c_item {
    int x;
};

item newItem(int x){
    item n = malloc(sizeof(struct c_item));
    n->x=x;
    return n;
}

int eq(item x, item y){
    if (x->x==y->x)
        return 1;
    return 0;
}

item input_item(){
    item n;
    printf("Inserisci x: ");
    scanf("%d", n->x);
    return n;
}

void output_item(item x){
    printf("x: %d\n", x->x);
}

item cloneItem(item x){
    item n ;
    n->x=x->x;
    return n;
}

queue.h

#include "item.h"

typedef struct queue *queuePtr;

queuePtr newQueue();
int emptyQueue(queuePtr q);
item dequeue(queuePtr q);
int enqueue(item val, queuePtr q);

void checkPointer(queuePtr p);

queue.c

#include <stdlib.h>
#include <stdio.h>
#include "item.h"
#include "queue.h"

typedef struct node* nodePtr;

struct node{
    item val;
    nodePtr next;
};

struct queue{
    nodePtr head, tail;
    int dim;
};

queuePtr newQueue(){
    queuePtr q = malloc(sizeof(struct queue));

    if (q==NULL)
        return NULL;

    q->dim=0;
    q->head=NULL;
    q->tail=NULL;

    return q;
}

int emptyQueue(queuePtr q){
    if (q==NULL)
        return -1;
    return q->dim==0;
}

// aggiunge un nodo alla coda
int enqueue(item val, queuePtr q){

    if (q==NULL)
        return -1;

    nodePtr nuovo = malloc(sizeof(struct node));

    if (nuovo==NULL)
        return 0;

    nuovo->val=val;
    nuovo->next=NULL;

    if(q->head==NULL) {
        q->head = nuovo;
    }
    else {
        q->tail->next = nuovo;
    } 
    q->tail=nuovo;
    (q->dim)++;
    return 1;

}

item dequeue(queuePtr q){
    if (q==NULL)
        return (item)NULL;

    if (q->dim==0)
        return (item)NULL;

    item res = q->head->val;

    struct node* tmt = q->head;

    q->head=q->head->next;

    free(tmt);

    if (q->head==NULL)
        q->tail=NULL;

    (q->dim)--;

    return res; 
}

main.c

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

int main() {

    queuePtr q = newQueue();

    item val = newItem(1);

    item val2 = newItem(2);

    enqueue(val, q);
    enqueue(val2, q);

    item ret = dequeue(q);
    printf("x: %d\n", ret->x);

    return 0;
}

But on compiling I have this error message:

/LibreriaQueque/main.c:17:26: error: dereferencing pointer to incomplete type ‘struct c_item’
 printf("x: %d\n", ret->x);

The IDE gives me the same alert message in the queue.c but it works. I think that the problem is the item structure that is declared in the item.c and so it can't be seen in the main file and in the queue.c file. I can't move the structure declaration from item.c. How can I solve this problem?

In main the internal structure of struct c_item is not known - only what a pointer to it is. If you want to keep the details hidden you could provide a function to return the value of each member. — Weather Vane
– Weather Vane, Commented Apr 24, 2018 at 8:30
In this case I have only an int but if i have more elements i have to create getter like in object oriented programing? @WeatherVane — Invictus97k
– Invictus97k, Commented Apr 24, 2018 at 8:34
@FRANCESCOPIOGAGLIONE by defining a "private" struct and an API working on that struct, you're effectively doing object-oriented programming. Of course it's your choice whether you want to hide the struct or not ... — user2371524
– user2371524, Commented Apr 24, 2018 at 8:35
typedef struct c_item *item; Don't typdef pointers... If you really must then use a name that makes it clear that it is a pointer. Like typedef struct c_item *itemPtr; — 4386427
– 4386427, Commented Apr 24, 2018 at 8:46
When writing an ADT, it is good practice to prefix all functions, types and macros in the same way. In this case, I would name all function item_ .... — Lundin
– Lundin, Commented Apr 24, 2018 at 9:53

user2371524 · Accepted Answer · 2018-04-24 08:31:17Z

You're doing an often sensible thing here: The struct is only defined in your implementation of the queue. This makes sure no other module can ever depend on the inner workings of your struct, so you can change it without touching any external code.

But in your main(), you try to do what you explicitly forbid: accessing the content of your struct. It's impossible because the compiler doesn't know the content when compiling your main file.

You have two options:

Move the definition of the struct to your header file, therefore making it public.
(IMHO preferred): Provide an accessor method like
```
int value(const item it)
{
    return it->x;
}
```
and call that from your main code to get the value.

Side notes:

What is memory.h? I guess you don't need it
Better don't hide pointers behind typedef. You could to typedef struct c_item item; instead and use the explicit asterisk anywhere. It's easier to understand the code, C programmers expect pointers to be explicit.

Lundin · Accepted Answer · 2018-04-24 10:43:13Z

You are close to using what's known as "opaque type" or "opaque pointers", which is a design pattern used for private encapsulation of ADTs. It works by declaring an incomplete struct in the header file, which the caller has access to. But only defining the struct in the c file, which the caller does not have access to. Therefore the caller can't know what's inside the struct, which is a good thing.

The downside of this method is that the caller can never create an instance of the object - only pointers to it. (It works pretty much like an abstract base class in C++.) So the caller has to rely on your "constructor" and "destructor" to do this.

Misc good practice:

There's a rule of thumb in C saying that you should never hide pointers behind typedefs, because doing so tends to confuse the reader.
When writing an ADT, it is good practice to prefix all functions, types and macros in the same way. In this case, I would name all functions queue_. Ideally you should have a coding standard stating how to prefix/postfix functions, macros and types belonging to an ADT.
You should never use empty parentheis for functions in C, because that's obsolete style and means "take any parameter". (Unlike in C++ where it means take no parameter.)
Use const correctness for the ADT function parameters.

Now to make your ADT opaque (which is good design), you need to do like this:

queue.h

typedef struct queue queue; // typedef of incomplete type

queue* queue_new (void);
void queue_delete (queue* q);
int queue_empty (const queue* q);
// and so on

queue.c

struct queue {
  ...
};

main.c

queue* q = queue_new();

Collectives™ on Stack Overflow

c programming, queue and modules problems

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related