Favolas
Favolas

Reputation: 7243

C Doubly linked list with structure

I'm doing a doubly linked list. As far as I know it is working but came here to make sure and to see if I'm doing the correct way.

On the other side, when I made this I came upon to other questions not related to doubly linked list but with structures and "visibility" between C files. If you understand that I should create other questions to this two other doubt's please tell. Otherwise feel free to enlighten me.

On my file1.c I have this:

CODE

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

typedef struct team{
    char *name;
    char *teamPlace;
}Team;

typedef struct nodeTeam{
    int numberOfTeams;
    Team team;
    struct nodeTeam *next;
    struct nodeTeam *prev;
}NodeTeam;

int createsListOfTeams(NodeTeam **head, NodeTeam **tail);
void printListOfTeams(NodeTeam *listofTeams);
int addNodeTeamsSorted(NodeTeam *head, NodeTeam **tail, Team team);

int main()
{
    NodeTeam *headEquipas,*tailEquipas;
    Team eq;
    /*Creates the doubly linked list*/
    if(createsListOfTeams(&headEquipas,&tailEquipas)){
        printf("\nError\n");
        return 0;
    }
    /*Add the teams to the doubly linked list. At the end, all teams will be sorted by name*/
    eq.name = "D team";
    eq.teamPlace = "D team place";
    if (addNodeTeamsSorted(headEquipas,&tailEquipas,eq)){
        printf("\nError\n");
        return 0;
    }

    eq.name = "A team";
    eq.teamPlace = "A team place";
    if (addNodeTeamsSorted(headEquipas,&tailEquipas,eq)){
        printf("\nError\n");
        return 0;
    }

    eq.name = "C team";
    eq.teamPlace = "C team place";
    if (addNodeTeamsSorted(headEquipas,&tailEquipas,eq)){
        printf("\nError\n");
        return 0;
    }

    eq.name = "B team";
    eq.teamPlace = "B team place";
    if (addNodeTeamsSorted(headEquipas,&tailEquipas,eq)){
        printf("\nError\n");
        return 0;
    }

    /*Will print all the teams*/
    printListOfTeams(headEquipas);

    return 0;
}

And on my file2.c I have this

CODE

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

    typedef struct team{
        char *name;
        char *teamPlace;
    }Team;

    typedef struct nodeTeam{
        int numberOfTeams;
        Team team;
        struct nodeTeam *next;
        struct nodeTeam *prev;
    }NodeTeam;

    /*Add the teams to the doubly linked list. At the end, all teams will be sorted by name*/
    int createsListOfTeams(NodeTeam **head, NodeTeam **tail){
        (*head) = (NodeTeam *)malloc(sizeof(NodeTeam));

        if ((*head) == NULL){
            return -1;
        }
        (*head)->numberOfTeams = 0;
        (*head)->team.teamPlace = "";
        (*head)->team.name = "";
        (*head)->next = NULL;
        (*head)->prev = NULL;

        *tail = *head;
        return 0;
    }

    /*Creates the doubly linked list*/
    int addNodeTeamsSorted(NodeTeam *head, NodeTeam **tail, Team team){
        NodeTeam *no,*listIni;


        no = (NodeTeam*) malloc(sizeof(NodeTeam));
        if (no == NULL){
            return -1;
        }

        /*copy of my list*/
        listIni = head;

        no->team = team;
        /*to see is it's the first element of my list*/
        if(head->numberOfTeams == 0)
        {
            no->next = head->next;
            no->prev = head;
            head->next = no;
            *tail = no;

        }
        else{ /*If not the first element*/
            head = head->next;
            while(head->prev != *tail && strcmp(head->team.name,no->team.name) < 0 && strcmp((*tail)->team.name,no->team.name)>0){
                head = head->next;
                (*tail) = (*tail)->prev;
            }
            if(strcmp(head->team.name,no->team.name) >= 0 || head->prev == *tail){
                no->next = head;
                no->prev = head->prev;
                (head->prev)->next = no;
                head->prev = no;

            }
            else if(strcmp((*tail)->team.name,no->team.name) <= 0){
                no->next = (*tail)->next;
                no->prev = (*tail);
                (*tail)->next = no;
                *tail = no;

            }
        }

        /*Updates the number of element of the list*/
        head = listIni;
        head->numberOfTeams++;

        return 0;
    }
    /*Prints my lists*/
    void printListOfTeams(NodeTeam *listofTeams){
        printf("|   number of teams %22d |\n",listofTeams->numberOfTeams);
        printf("|      team name      |        team place      |\n");
        printf("--------------------------------------------------\n");
        listofTeams = listofTeams->next;
        while (listofTeams != NULL){
            printf("| %-21s | %-22s |\n",listofTeams->team.name,listofTeams->team.teamPlace);
            listofTeams = listofTeams->next;
        }
        printf("--------------------------------------------------\n\n");
    }

So here are my tree questions:

Q1 - Is this the correct way to implement a doubly linked list with an head and a tail pointing to the beginning and the end of the list respectively?

Q2 - Why do declare on both of my files the struct team and the struct nodeTeam? Since they are all in the same project should't the declaration be "visible" to all of the files on my project?

Q3 - In struct teamwhy do I have to declare char *name instead of char name[31]?

Upvotes: 2

Views: 3361

Answers (1)

Seki
Seki

Reputation: 11465

I made some edits after your previous comments and after analyzed your code more carefully. I wrongly interpreted one remark about the head and tail items and though you were designing a circular list

  1. I took the time to copy / paste / compile your code. While it is almost working I must say that I would have designed in another way by

    • moving the prev/ next pointers into the struct team
    • and replacing the team member of nodeTeam by a head pointer to the first team.

    That would have the following benefits :

    • prevent a useless waste of space for the numberOfTeams that is duplicated for each nodeTeams but only meaningful to the first
    • avoid a confusion between the conceptual head and the actual first team

    By adding the value of the pointers in the team list with

    printf("| %-21s | %-22s | %p - p=%p n=%p\n",listofTeams->team.name, listofTeams->team.teamPlace, listofTeams, listofTeams->prev, listofTeams->next);

    I noticed a possible bug in your linking :

    | A team | A team place | 0x101d00980 - p=0x101d00920 n=0x101d009e0

    | B team | B team place | 0x101d009e0 - p=0x101d00980 n=0x101d009b0

    | C team | C team place | 0x101d009b0 - p=0x101d00980 n=0x101d00950

    | D team | D team place | 0x101d00950 - p=0x101d009b0 n=0x0

    You can see that next pointers are ok, but the prev pointers show suspicious duplicates (0x101d00920 is indeed the 'head').

    If you trace the execution of your code and check what it done in addNodeTeamsSorted() You could notice that everything is ok until step 3 (addition of team C, after existing A & D) :

    • due to the weird double modification of both head and tail to find the place to insert the new item, head an tail are crossing: tail actually points to 'A' and head to 'D' (dont forget that while head is Nodeteam * and its modification won't be propagated outside of the function, tail is a Nodeteam **, thus when it is changed into the caller and for the next call it will be wrong
    • in the step 4 (addition of 'B') in the else if part of the test, you change properly the prev/next of no, the next of (*tail) but not prev of no->next so you have
      • 'B' -> next = 'C' : OK
      • 'B' -> prev = 'A' : OK
      • *tail (='A') -> next = 'B' : OK
      • 'C' -> prev still = 'A' : WRONG
  2. This is not like that that the compiler is thinking. He only processes compilational units, one at a time. What is not declared in the .c and the different included .h will remain unknown. If you want to share the structs declarations between 2 modules, and to prevent errors in code maintenance, cut the typedefs and put them in a common header file (e.g. equipa.h) that will be included in both .c

  3. You have to use char* instead of char[] because in your main() in file1.c you are making direct assignments from literal strings and the compiler won't let you assign a literal string to a char array. If you wan to use char[] instead change your

    eq.nome = "D team";

    by string copying like

    strcpy(eq.nome, "D team");

    of course, I am only dealing with the concept, actually you should take care that the string that will be copied are not longer than the buffer by using strncpy() and sizeof(eq.nome)

Upvotes: 2

Related Questions