Leonhard
Leonhard

Reputation: 11

How can I link a data structure with another?

I'm trying to make a shop system. I have a linked list and a stack. The linked list store the Person information (id and name) and the Stack store the Products (product).

I need to link both of them but I don't know how. I want to create some Person and the person will choose some products like:

Person1:

Person2:

Every person will have their chosen products.

Here's my code so far:

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

#define TAM 30

typedef int ID;
typedef char PRODUCT[TAM];

typedef struct stack{
    int nItems;
    int capacity;
    PRODUCT *product;
} Stack;

typedef struct person{
    Stack *stack;
    ID id;
    char name[TAM];
    struct person *next;
}Person;

typedef struct list{
    Person *head;  
    Person *end;
}List;

void createList(List *list){
    list->head = NULL;
    list->end = NULL;
}

void insert(List *list, Person person){

    Person *newPerson = (Person*)malloc(sizeof(Person));
    Person *ant = NULL;
    Person *a = list->head;
    strcpy(newPerson->name, person.name); 
    newPerson->id = person.id;

    while(a != NULL && a->id < a->id){
        ant = a;
        a = a->next;
    }
    if (ant == NULL){
        newPerson->next = list->head;
        list->head = newPerson;
    }else{
        newPerson->next = ant->next;
        ant->next = newPerson;
    }
}

void printList(List *list){

    Person *a;
    a = list->head;

    while(a != NULL){ 
        printf("ID: %d | Name: %s\n", a->id, a->name);
        a = a->next;
    }
}

Stack *createStack(int capacity){

    Stack *stack = malloc(sizeof(Stack));
    stack->product = malloc(sizeof(int) * capacity); 
    stack->capacity = capacity;

    return stack;
};

bool push(Stack *stack, PRODUCT product){
    if (stack->capacity == stack->nItems){
        return false;
    }

    strcpy(stack->product[stack->nItems], product);

    return true;
}

void printStack(Stack *stack){
    int i;

    for(i = 0; i < stack->nItems; i++){
        printf("product: %s\n", stack->product[stack->nItems - i - 1]);
    }
}

int main(){

    int capacity = 10;
    Stack *stack = createStack(10);
    List list;

    Person p1 = {1, "James"};
    PRODUCT prod1 = {"Apple"};

    createList(&list);
    insert(&list, p1);

    push(stack, prod1);

    printList(&list);
    printStack(stack);
    return 0;
}

Upvotes: 1

Views: 46

Answers (1)

David C. Rankin
David C. Rankin

Reputation: 84521

You can do exactly what you are attempting to do, but you have missed the point of Person including Stack *stack; by creating a separate Person and Stack in main(). There is no relationship between the two if you create a separate Person and a separate Stack. Because Person includes a pointer to stack, you must create the stack when you insert() the person.

You are thinking along the correct lines in how you approach the list and stack, you are just struggling to get the pieces put together correctly and in the right order.

Let's start with your attempt in main() to create p1 which you want to use as a temporary struct to insert. You declare and initialize Person p1 = {1, "James"};, but the first member in Person is Stack *stack;. So, of course, the compiler chokes when it sees 1 as the initializer for Stack *stack;. You can make your wanted initialization work by moving Stack *stack; to the 3rd member in the struct, e.g.

typedef struct person {
    ID id;
    char name[TAM];
    Stack *stack;               /* move to 3rd member */
    struct person *next;
} Person;

When you allocate memory, you must validate EVERY allocation. If there are multiple allocations in any one function, if the 2nd (or later) allocation fails, you must clean up (free()) the prior allocations before returning an error condition. You can see that in createStack(), e.g.

Stack *createStack (int capacity)
{
    Stack *stack = malloc (sizeof *stack);
    
    if (!stack) {   /* validate EVERY allocation */
        perror ("malloc-stack");
        return NULL;
    }
    
    stack->product = malloc (capacity * sizeof *stack->product); 
    
    if (!stack->product) {  /* validate EVERY allocation */
        perror ("malloc-stack->product");
        free (stack);       /* free stack memory on failure */
        return NULL;
    }
    
    stack->capacity = capacity;
    stack->nItems = 0;    /* don't forget to initialize nItems 0 */
    
    return stack;
}

As mentioned above, since the Stack *stack; is a member of Person, you must create the stack as part of creating the Person. (you can do it in separate functions if you like, but you create Person->stack NOT some separate unrelated stack). It is common to do it in the same function where you create your Person, e.g.

Person *insert (List *list, const Person *person, int capacity)
{
    Person *newPerson = malloc (sizeof *newPerson);
    
    if (!newPerson) {   /* validate EVERY allocation */
      perror ("malloc-newPerson");
      return NULL;
    }
    
    *newPerson = *person;       /* assign automatic struct members */
    
    /* create / validate stack */
    newPerson->stack = createStack (capacity);
    if (!newPerson->stack) {
        free (newPerson);       /* free newPerson memory on failure */
        return NULL;
    }
    
    newPerson->next = NULL;     /* initialize pointers NULL */
    
    if (!list->head) {          /* good job on the end ptr, now use it */
        list->head = list->end = newPerson;
    }
    else {
        list->end->next = newPerson;
        list->end = newPerson;
    }
    
    return newPerson;
}

(NOTE: insert cannot be type void. You must return a type that indicates whether your allocation within the function succeeded or failed. Simply returning the pointer you create in the function (or NULL on failure) works fine.)

Your push() function should provide a meaningful error if the stack is full, e.g.

bool push (Stack *stack, const PRODUCT product)
{
    if (stack->capacity == stack->nItems) {
        /* provide meaningful error messages */
        fprintf (stderr, "error: failed to add: %s (stack full)\n",
                 product);
        return false;
    }

    strcpy (stack->product[stack->nItems], product);
    stack->nItems++;    /* don't forget to increment nItems */

    return true;
}

(note: the addition of the increment of stack->nItems, without it you just keep overwriting the first product...)

Since your stack is part of Person, you need to printStack() as part of printList(), e.g.

void printStack (Stack *stack)
{
    int i;
    
    if (!stack->nItems) {       /* don't forget to handle empty-stack */
        puts ("stack-empty");   /* or your printf() ivokes UB */
        return;
    }
    
    for (i = 0; i < stack->nItems; i++) {
        printf ("  product: %s\n", stack->product[stack->nItems - i - 1]);
    }
}

void printList (List *list){

    Person *a = list->head;

    while (a != NULL) { 
        printf ("\nID: %d | Name: %s\n", a->id, a->name);
        
        printStack (a->stack);
        
        a = a->next;
    }
}

(note: the addition of the validation in printStack() to avoid Undefined Behavior by referencing a negative index if the stack is empty)

(also note: there is no reason to print the stack in reverse order -- unless you just want to. You are adding the product at stack->nItems each time so they will be in the order added)

In any code you write that dynamically allocates memory, you have 2 responsibilities regarding any block of memory allocated: (1) always preserve a pointer to the starting address for the block of memory so, (2) it can be freed when it is no longer needed. You do fine with (1), but completely fail doing (2).

Yes, since your list is created in main(), there is no technical memory-leak since the memory will be freed on program exit, but you will not always be creating lists in main(). In that case, if you fail to free() before leaving the function you created in -> memory-leak. Build good habits early, always track and free what you allocate. A simple routine to free your stack and Person nodes in your list could be:

void del_stack (Stack *stack)
{
    free (stack->product);
    free (stack);
}

void del_list (List *list)
{
    Person *person = list->head;
    
    while (person) {
        Person *victim = person;
        del_stack (victim->stack);
        
        person = person->next;
        free (victim);
    }
}

Now you can simply call del_list (&list); to free all allocated memory.

Lastly, be very careful trying to be too creative with typedef. Never typedef pointers and make sure you understand a typedef of arrays. In your code you have:

typedef char PRODUCT[TAM];

and then in Stack you have:

    PRODUCT *product;

What is product? Is it a pointer-to-array or is it an array-of-pointers? (are you sure?) (hint: you have single-allocation, single-free, so it can only be one of the two)

Now in main(), it is fine to use a temporary pointer to Person to initialize and then insert in your list. But don't confuse the temporary Person with the node created in the list. Putting all the above together, and intentionally attempting to insert 12 products where only 10 will fit to exercise the checks in the code, you could do:

int main() {

    int capacity = 10;
    List list;
    PRODUCT prod = "";
    
    createList (&list);
    
    Person tmp = { .id = 1, .name = "James"};
    
    /* must create stack with person */
    Person *p1 = insert (&list, &tmp, capacity);
    
    if (!p1) {      /* validate node & stack creation succeeds */
        return 1;
    }

    for (int i = 0; i < capacity + 2; i++) {
        sprintf (prod, "Product_%d", i + 1);
        if (!push (p1->stack, prod)) {
            break;
        }
    }

    printList (&list);
    del_list (&list);     /* don't forget to free what you allocate */
}

Example Use/Output

$ ./bin/llstack
error: failed to add: Product_11 (stack full)

ID: 1 | Name: James
  product: Product_10
  product: Product_9
  product: Product_8
  product: Product_7
  product: Product_6
  product: Product_5
  product: Product_4
  product: Product_3
  product: Product_2
  product: Product_1

Memory Use/Error Check

It is imperative that you use a memory error checking program to ensure you do not attempt to access memory or write beyond/outside the bounds of your allocated block, attempt to read or base a conditional jump on an uninitialized value, and finally, to confirm that you free all the memory you have allocated.

For Linux valgrind is the normal choice. There are similar memory checkers for every platform. They are all simple to use, just run your program through it.

$ valgrind ./bin/llstack
==7287== Memcheck, a memory error detector
==7287== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==7287== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==7287== Command: ./bin/llstack
==7287==
error: failed to add: Product_11 (stack full)

ID: 1 | Name: James
  product: Product_10
  product: Product_9
  product: Product_8
  product: Product_7
  product: Product_6
  product: Product_5
  product: Product_4
  product: Product_3
  product: Product_2
  product: Product_1
==7287==
==7287== HEAP SUMMARY:
==7287==     in use at exit: 0 bytes in 0 blocks
==7287==   total heap usage: 4 allocs, 4 frees, 1,396 bytes allocated
==7287==
==7287== All heap blocks were freed -- no leaks are possible
==7287==
==7287== For lists of detected and suppressed errors, rerun with: -s
==7287== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Always confirm that you have freed all memory you have allocated and that there are no memory errors.

Complete Example Code

For easy reference, here are the complete modifications to your code:

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

#define TAM 30

typedef int ID;
typedef char PRODUCT[TAM];

typedef struct stack {
    int nItems;
    int capacity;
    PRODUCT *product;
} Stack;

typedef struct person {
    ID id;
    char name[TAM];
    Stack *stack;               /* move to 3rd member */
    struct person *next;
} Person;

typedef struct list {
    Person *head;  
    Person *end;
} List;

void createList (List *list) 
{
    list->head = NULL;
    list->end = NULL;
}

Stack *createStack (int capacity)
{
    Stack *stack = malloc (sizeof *stack);
    
    if (!stack) {   /* validate EVERY allocation */
        perror ("malloc-stack");
        return NULL;
    }
    
    stack->product = malloc (capacity * sizeof *stack->product); 
    
    if (!stack->product) {  /* validate EVERY allocation */
        perror ("malloc-stack->product");
        free (stack);       /* free stack memory on failure */
        return NULL;
    }
    
    stack->capacity = capacity;
    stack->nItems = 0;    /* don't forget to initialize nItems 0 */
    
    return stack;
}

Person *insert (List *list, const Person *person, int capacity)
{
    Person *newPerson = malloc (sizeof *newPerson);
    
    if (!newPerson) {   /* validate EVERY allocation */
      perror ("malloc-newPerson");
      return NULL;
    }
    
    *newPerson = *person;       /* assign automatic struct members */
    
    /* create / validate stack */
    newPerson->stack = createStack (capacity);
    if (!newPerson->stack) {
        free (newPerson);       /* free newPerson memory on failure */
        return NULL;
    }
    
    newPerson->next = NULL;     /* initialize pointers NULL */
    
    if (!list->head) {
        list->head = list->end = newPerson;
    }
    else {
        list->end->next = newPerson;
        list->end = newPerson;
    }
    
    return newPerson;
}

bool push (Stack *stack, const PRODUCT product)
{
    if (stack->capacity == stack->nItems) {
        /* provide meaningful error messages */
        fprintf (stderr, "error: failed to add: %s (stack full)\n",
                 product);
        return false;
    }

    strcpy (stack->product[stack->nItems], product);
    stack->nItems++;    /* don't forget to increment nItems */

    return true;
}

void printStack (Stack *stack)
{
    int i;
    
    if (!stack->nItems) {       /* don't forget to handle empty-stack */
        puts ("stack-empty");   /* or your printf() ivokes UB */
        return;
    }
    
    for (i = 0; i < stack->nItems; i++) {
        printf ("  product: %s\n", stack->product[stack->nItems - i - 1]);
    }
}

void printList (List *list){

    Person *a = list->head;

    while (a != NULL) { 
        printf ("\nID: %d | Name: %s\n", a->id, a->name);
        
        printStack (a->stack);
        
        a = a->next;
    }
}

void del_stack (Stack *stack)
{
    free (stack->product);
    free (stack);
}

void del_list (List *list)
{
    Person *person = list->head;
    
    while (person) {
        Person *victim = person;
        del_stack (victim->stack);
        
        person = person->next;
        free (victim);
    }
}

int main() {

    int capacity = 10;
    List list;
    PRODUCT prod = "";
    
    createList (&list);
    
    Person tmp = { .id = 1, .name = "James"};
    
    /* must create stack with person */
    Person *p1 = insert (&list, &tmp, capacity);
    
    if (!p1) {      /* validate node & stack creation succeeds */
        return 1;
    }

    for (int i = 0; i < capacity + 2; i++) {
        sprintf (prod, "Product_%d", i + 1);
        if (!push (p1->stack, prod)) {
            break;
        }
    }

    printList (&list);
    del_list (&list);     /* don't forget to free what you allocate */
}

Let me know if you have further questions.

Upvotes: 1

Related Questions