1

The idea: a list of lists (for a publisher-suscriber problem)

The problem: the lists use different types of structs as nodes. So my function does not work with the second type of struct (that's just fine, I could simply create another function and just change the parameters that it uses).

But I feel that's a rather simplistic approach and honestly the amount of code I'm beggining to handle is a bit too much.

Is there a more professional/experienced way to do this? (that I can handle. For example here they talk about it but I'm not quite sure I could implement it without messing up since I've never used unions:

How to Pop different types of Structs from a Stack )

Is this a common/aceptable way of doing things (reusing functions for different structs/data types)?

Structs I use:

struct nodoTemas{
    char * nombreTema;
    struct nodoSuscriptor * nodoCerodeSuscriptores;
    struct nodoTemas * next;
};

struct nodoSuscriptor{
    char * nombreSuscriptor;
    //int iP;
    struct nodoTemas * next;
};

So I have a working code with a function that creates the list , and other similar methos to interact with it.

struct nodoTemas* create_list(char * val, struct nodoTemas * head, struct nodoTemas *curr)
 {
     printf("\n creating list with headnode as [%s]\n",val);
     struct nodoTemas *ptr = (struct nodoTemas*)malloc(sizeof(struct nodoTemas));
if(NULL == ptr)
{
    printf("\n Node creation failed \n");
   return NULL;
}
ptr->nombreTema = val;
ptr->next = NULL;

head = curr = ptr;
printf("Ha llegado aqui3\n");
return ptr;
 }

I provided this much code because I'm usually asked to do so. As always I'm sorry if this is not the right place/the question is not worded properly.

EDIT: I have now find out that with union and struct, it'd hold space for the biggest of the two or more types. I am not sure if this is just wasting too much space and thus making that a not so good option, so not quite sure how to do it (the idea being that if if it's a list of suscribers, it could be 2 or 2000, and with each node added there'd be wasted space).

keont
  • 653
  • 1
  • 9
  • 26
  • the line `head=curr=ptr` is complete nonsense, because you pass the head and current by value, you you do not change those values outside the scope of you function create(). – Peter Miehle Mar 31 '15 at 08:48
  • I'm not sure if I understand you but in other parts of the code I do change them. That function was just an example. If you mean that by itself is nonsense, would you mind explaining why? Because no idea, I tried out different things and if I remove that I'm pretty sure it stops working so... – keont Mar 31 '15 at 09:02
  • 1
    what do you intent to try with that line of code? your function abstracted: `void *fun(void *head) {void *ptr = malloc(); head=ptr; return ptr;}`. So what is the line `head=ptr;` doing? it does not change anything outside the scope of `fun()` and it is the but-last statement inside `fun()` with no side-effect. so I think, you intent something to do with that line, and it does not what you want it to do, like changing the head (and curr) OUTSIDE the scope of `fun()` like in `create_list("text", myhead, mycurr);` myhead and mycurr are not changed by the call. – Peter Miehle Mar 31 '15 at 10:09
  • aaaaaaaah damn. Allright this is why I still suck doing versions of code. Originally it weren't variables, they were globals. I kind of guessed it was a silly thing to do if my code was going to involve more than one list so I made them local. And so I changed the function to work with a head and a current being passed, rather that just getting the value because global. So thanks for taking the time to explain, I was mixing stuff up. – keont Mar 31 '15 at 10:37

2 Answers2

2

The problem you are struggling with - how to create reusable generic functionality, such as containers - is one which is addressed easily with object oriented programming languages such as C++. However they can be implemented in C, even if not as easily and flexibly.

The key is to identify which functionality is common and generic, and which is specific to the types of nodes in the list.

Generic functionality would be the functions such as creating/initializing the list, adding nodes, deleting nodes, iterating through the list, etc.

Create stand alone functions for handling lists, together with a struct that models the generic nodes.

Then, create structures representing the different object types in the list. Embed one of the generic list node structs as the first element in each of these structs, and write functions as needed to provide the functionality for handling each of the different types of objects you need to deal with.

In that way you end up with generic, re-usable lists, and as an added bonus you keep your code for dealing with the specific object types clean and easily maintainable.

Here is a simple program I whipped up to demonstrate the principle. It allows creating a list of objects representing apples and oranges. The apples have functionality for tracking their weight, and the oranges have functionality for tracking their price.

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

/* First we start with the definition of a generic list node */

struct list_node {
    struct list_node *next;
    struct list_node *prev;

    /* The 'type' field is important - it allows us to have a list
     * containing a number of different types of node, and to be able
     * to find out what type each node is */
    int type;
};

/* Some variables to keep track of the beginning and end of the list. */

struct list_node* head;
struct list_node* tail;

/* Now some generic functions for dealing with lists - initializing the list,
 * adding nodes through it, iterating through the list */

void list_init() {
    head=tail=NULL;
}

void list_add_node(struct list_node* node) {
    if (NULL==tail) {
        head=tail=node;
        node->next=NULL;
        node->prev=NULL;
    } else {
        tail->next=node;
        node->next=NULL;
        node->prev=tail;
        tail=node;
    }
}

struct list_node* list_get_next(struct list_node* node) {
    if (NULL==node)
        return head;
    else
        return node->next;
};

/* Great, now we have a generic set of functions for dealing with generic lists.
 * But how do we use that to contain different kinds of objects? The answer
 * is composition - we include the list_node as the first element of each of the
 * structs that we want to add to the list
 *
 *
 * Here we define a struct for recording the weight of apples, together with
 * functions specific to dealing with apples
 */

struct apple {
    struct list_node node;
    int weight;
};

struct apple* new_apple(int weight) {
    struct apple* a = (struct apple*)malloc(sizeof(struct apple));
    /* Apples are considered type 1 */
    a->node.type = 1;
    a->weight = weight;
    return a;
};

void apple_printweight(struct apple* a) {
    printf("This is an apple and it weighs %d grams\n", a->weight);
}

/* And here we define a struct for recording the price of oranges, together with
 * functions specific for dealing with oranges
 */

struct orange {
    struct list_node node;
    double price;
};

struct orange* new_orange(double price) {
    struct orange* o = (struct orange*)malloc(sizeof(struct orange));
    /* Oranges are type 2 */
    o->node.type = 2;
    o->price=price;
    return o;
};

void orange_printprice(struct orange* o) {
    printf("This is an orange and it costs $%6.2f dollars\n", o->price);
}

/* Now to use our oranges and apples */

int main() {

    list_init();

    /* Create an orange, add it to the list
     * 
     * Note the need to cast the orange to a list_node
     * so we can call the 'list_add_node' function.
     * This makes use of a property of pointers to structs:
     * you can always convert them to point to the first element of
     * the struct. 
     */
    struct orange* myOrange = new_orange(12.50);
    list_add_node((struct list_node*)myOrange);

    /* Create an apple, add it to the list */
    struct apple* myApple = new_apple(150);
    list_add_node((struct list_node*)myApple);

    /* Iterate through the list */
    struct list_node* n = NULL;
    while (n = list_get_next(n)) {

        /* For each node we come to, it could be an apple or an orange.
         * Inspect the type to find out what type it is, and use it
         * accordingly */
        if (n->type == 1) {
            apple_printweight((struct apple*)n);
        } else if (n->type == 2) {
            orange_printprice((struct orange*)n);
        }
    }

    /* In a real program you would want to free the list objects here
     * to avoid a memory leak
     */

}

Here is the output:

This is an orange and it costs $ 12.50 dollars
This is an apple and it weighs 150 grams

Bear in mind that this is just a simple example, and by no means does it illustrate best practice in all aspects. In real life you would separate the declarations into header files, and you would probably do something more elegant than the if/then branching in the main loop to handle the different types.

harmic
  • 28,606
  • 5
  • 67
  • 91
  • As you said, the fact I even thought of doing this is because of object oriented programming (Java). I'm trying to understand what is being explained to me, but I still have a few problems. For starters, if the nodes were vastly more different between each other, would this still work? The logical answer is yes because I do not find any reason in the code as to why they wouldn't. But I can't quite see WHY they would work. Second, to add myOrange to the list, you cast it as a node. I... don't quite see it. I've read you can cast almost anything to almost anything, but what about their sizes. – keont Apr 02 '15 at 06:59
0

I'm struggling to find a reason why you're casting mallocs return value. It seems, if you want C++ compatibility, just nuke C and adopt C++'s linked lists and your life gets so much easier. As you're about to see, generics in C can be a pain in the butt! If you want to write C code, nuke the casts. If you want to reuse the C code in a C++ project, compile it using a C compiler and link to it using the compatible C++ compiler.

Yes, you'll want to eliminate redundant code. The first step to making your life that much easier is to eliminate malloc from your linked list code. Pass all allocated data in via arguments, just like sprintf requires. There is a reason sprintf was designed like that. Liberation of data-specific context goes hand-in-hand with liberation of allocation. That probably means your linked list struct will contain only one member: A pointer to the next node, eg struct linked_list_node { struct linked_list_node *next; };... So be it! You can declare new list types like so, and use the functions you've created to handle the generic list type:

struct apple_linked_list {
    struct linked_list_node linked_list;
    char *colour;
};

struct apple_linked_list_linked_list {
    struct linked_list_node super_list;
    struct apple_linked_list_node *sub_list;
};

struct apple_linked_list red = { .colour = "red" },
                         blue = { .colour = "blue" },
                         green = { .colour = "green" },
                        *root = NULL;

root = linked_list_add(root, &red);
root = linked_list_add(root, &blue);
root = linked_list_add(root, &green);

struct apple_linked_list_linked_list list_list = { .sub_list = root };
struct apple_linked_list_linked_list *super_root = NULL;
super_root = linked_list_add(super_root, list_list);

So now you want malloc back. I can't say I blame you. It is a useful tool... but it's not to be inserted into your existing add function, because it's also useful to be able to create non-malloc'd lists. Kinda like how it's useful to be able to use sprintf without requiring malloc or free... Write a new struct to extend the old, and a new set of functions to operate on this struct. This gets a little more complex, but it's still possible. It might also be sensible to rename your previous types and functions to automatic_linked_list.

I've written all of this code and tested it, and I'm prepared to offer it to the public domain here.

Hope this helps :)

autistic
  • 1
  • 3
  • 35
  • 80
  • dude... I'm sorry but I just don't have the level to follow you. Is this pure C? There's a lot of sintaxis I don't get. I'm sorry but could I make a few questions or it would be too much of a bother? I mean not even straight up copying your code I could use it, that's the problem. I'm usually slow but this is another level. First of all, linked_list_node. Where does this come from. Same with apple_linked_list_node. Structs appearing outa nowhere. Second. I don't think I'm casting malloc returns value. I'm casting malloc so it gives me chuncs of struct side. Isn't it? – keont Mar 31 '15 at 14:56
  • and I'm sorry again but I really don't get the not putting malloc in the add_list. I need memory space for the struct I'm creating right? And I need to ask for that new space. And I don't see the alternative way you say of doing. – keont Mar 31 '15 at 15:10
  • Yes, it's pure C. Don't ask to ask. Just ask. In my gist I've renamed `linked_list_node` to `automatic_linked_list_node`, and it comes from the header `linked_list.h`. `apple_linked_list_node` is a custom type using `linked_list_node`, like when you declare `list` in some other language. [Do I cast the result of malloc?](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc)... Yes, you need memory space for the struct you're creating... just like `sprintf` needs memory space for the string it creates... and you can get that from an argument, can't you? – autistic Mar 31 '15 at 18:15