Thread: Help with structures and linked lists.

  1. #1
    Registered User
    Join Date
    Oct 2012
    Posts
    23

    Help with structures and linked lists.

    Here is my problem: Write a program that utilizes a structure and linked list to store employee information taken from the keyboard (first name, last name, three digit employee id number). The program should traverse the linked list and display the information for the employee with the highest id number.


    I'm having troubles with the output and getting this to work in general. I'm not great with structures or linked lists so any help would be appreciated.

    Code:
    #include <stdio.h>
    #include <stdlib.h>
     
    typedef struct employee
    {char fname[22];
     char lname[22];
     int  id;
     struct employee*next;
    }WORK;
     
    WORK* insertit(WORK*first, char empFNAME[], char empLNAME[], int idnum);
     
    int main (void)
    {  WORK *head,
            *p;
       char namef[22],
            namel[22];
       int empID,
           j,
           k;
       char ch,
            nl;
     
    head=NULL;
    for(k=0; k<21; k++)
      { namef[k]=' ';
        namel[k]=' ';
      }
    for(j=0; j<4; j++)
       {printf("Enter the first name of the employee: ");
        scanf("%s", &namef);
        printf("\nEnter the last name of the employee: ");
        scanf("%s", &namel);
        printf("\nEnter the three digit ID number: ");
        scanf("%d", &empID);
        printf("\n");
        head=insertit (head, namef, namel, empID);
    for(k=0; k<21; k++)
       namef[k]=' ';
       namel[k]=' ';
       }
     
    p=head;
    while (p!=NULL)
      {puts(head->fname);
       puts(head->lname);
       printf("The ID number is: ", head->id);
       p=p->next;
      }
     printf("Hit any character to continue");
     scanf("%c", &ch);
    }
     
    WORK* insertit (WORK *first, char empFNAME[], char empLNAME[], int idnum)
    {WORK *p,
          *q,
          *newp;
     int found,
         len,
         i;
         found=0;
         q=first;
         p=first;
     
    while ((p!=NULL) && (!found))
      {if (p->id<idnum)
         {q=p;
          p=p->next;
         }   
        else
          found=1;
      }
     
    newp=(WORK*) malloc(sizeof(WORK));
    newp->id=idnum;
    strncpy(newp->fname, empLNAME, 21);
     
    newp->next=p;
    if(q!=p)
      q->next=newp;
    else
      first=newp;
    return (first);
    }

  2. #2
    Lurking whiteflags's Avatar
    Join Date
    Apr 2006
    Location
    United States
    Posts
    9,613
    I'll give it a quick look.

    Code:
            printf("Enter the first name of the employee: ");
            scanf("%s", &namef);
            printf("\nEnter the last name of the employee: ");
            scanf("%s", &namel);
            printf("\nEnter the three digit ID number: ");
            scanf("%d", &empID);
    scanf will convert text input and store it in a pointer. So you don't need to use the address-of (&) operator in front of a character array. You can read about this point here.

    I don't know why you are focusing on clearing arrays with code like this:
    Code:
            for(k=0; k<21; k++)
                namef[k]=' ';
            namel[k]=' ';
    1. A string would normally be cleared with '\0' not the space ' ' character.
    2. why is the size of the array always hard coded?
    3. Even if you insisted on doing this, it is a unit of work separate from everything else you are doing with the list. So, you should write a function or call
    Code:
    memset(namel, '\0', 21);
    so everything stays neat.

    Finally, here are the warnings and errors your code generated for me:
    Code:
    ||=== bar, Debug ===|
    C:\Users\Josh2\Documents\bar\main.cc||In function 'int main()':|
    C:\Users\Josh2\Documents\bar\main.cc|27|warning: format '%s' expects type 'char*', but argument 2 has type 'char (*)[22]'|
    C:\Users\Josh2\Documents\bar\main.cc|29|warning: format '%s' expects type 'char*', but argument 2 has type 'char (*)[22]'|
    C:\Users\Josh2\Documents\bar\main.cc|43|warning: too many arguments for format|
    C:\Users\Josh2\Documents\bar\main.cc|18|warning: unused variable 'nl'|
    C:\Users\Josh2\Documents\bar\main.cc||In function 'WORK* insertit(WORK*, char*, char*, int)':|
    C:\Users\Josh2\Documents\bar\main.cc|68|error: 'strncpy' was not declared in this scope|
    C:\Users\Josh2\Documents\bar\main.cc|53|warning: unused variable 'len'|
    C:\Users\Josh2\Documents\bar\main.cc|53|warning: unused variable 'i'|
    C:\Users\Josh2\Documents\bar\main.cc|50|warning: unused parameter 'empFNAME'|
    ||=== Build finished: 1 errors, 7 warnings ===|
    There are too many unused variables and a missing library. I think you need to fix these things first, so that I can compile something, then we'll see if your linked list works.

    SourceForge.net: Indentation - cpwiki

    edit: I decided to attach a file so that you know how I indented your code and so that the error messages line up.
    Attached Files Attached Files
    Last edited by whiteflags; 12-10-2012 at 02:08 PM.

  3. #3
    Registered User
    Join Date
    Oct 2012
    Posts
    23
    It compiles and runs for me and lets me enter some employee details but the problem I'm having is the output isn't coming out correctly. It seems to pick a random first name from one of the employees I enter and prints:
    "The ID number is: Tom
    The ID number is: Tom
    The ID number is: Tom
    The ID number is: Hit any key to continue"
    and then the program ends. I'm not great with C, I've been barely getting by this past semester and this is my final project for my class.

  4. #4
    Lurking whiteflags's Avatar
    Join Date
    Apr 2006
    Location
    United States
    Posts
    9,613
    It compiles and runs for me
    I get that a lot. The thing is, people don't know how to use their compilers. You should be compiling on the strictest warning level possible to catch the kind of mistakes I pointed out. That's why I compile the way that I do. Plus I pointed out other mistakes that you should correct, I wasn't taking offense to anything that you posted.

    Part of the reason your code worries me is because of all the unused variables. Like forgetting to use "empFNAME" seems to be important. I could delete them, but I don't know what your intentions are.

    Anyway, with some of the changes I asked you to make, I got farther than you did. This is the output I'm getting:

    Enter the first name of the employee: Tom

    Enter the last name of the employee: Smith

    Enter the three digit ID number: 11110

    Enter the first name of the employee: Jane

    Enter the last name of the employee: Doe

    Enter the three digit ID number: 22220

    Enter the first name of the employee: John

    Enter the last name of the employee: Public

    Enter the three digit ID number: 33330

    Enter the first name of the employee: Oliver

    Enter the last name of the employee: Warbucks

    Enter the three digit ID number: 44440

    Smith
    sh2
    The ID number is: Smith
    sh2
    The ID number is: Smith
    sh2
    The ID number is: Smith
    sh2
    The ID number is: Hit any character to continue

    So if you can consider that positive progress...
    Attached Files Attached Files

  5. #5
    Registered User
    Join Date
    Oct 2012
    Posts
    23
    I adapted the code I'm using from a text book example, I tried to modify to make it fit for my project so that would explain the unused variables.

  6. #6
    Lurking whiteflags's Avatar
    Join Date
    Apr 2006
    Location
    United States
    Posts
    9,613
    I'm going to move on now, but you really should fix the things I pointed out. I'm also going to recommend that you rewrite the insert function a certain way.

    Well if you want to make a sorted list by id number you need 2 things, a pointer to the head of the list and a new node, filled with data. Then you can do:

    Code:
    1. Declare pointer walk
    
    2. Set walk to the head of the list
    
    3. while ( walk->next != NULL && walk->next->id < new node->id ):
    
        3a. walk = walk->next
    
    4. new node->next = walk->next
    
    5. walk->next = new node
    That is for the case where the list is not empty.

    When the list is empty the function does trivial work, i.e. the new node is the head of the list, and new node->next should be set to NULL. I don't think anything else is wrong.

    I don't think it makes a difference if you allocate "new node" in the insert function or not, it's just that I wouldn't.

  7. #7
    Registered User
    Join Date
    Oct 2012
    Posts
    23
    I've been making lots of modifications to the code but I still can't get this to work right. Can anyone help?

    Code:
    #include <stdio.h> 
    #include <stdlib.h> 
    #include <string.h>
    
     typedef struct employee {
        char fnp[25];
        char lnp[25];
        int id;
        struct employee *next;
    }
    job;
    
    job* insert(job *first, char fnp[], char lnp[], int idnum);
    
    int main (void)
     {
        job *head, *p;
        char enf[21];
        char enl[21];
        int empID, b, i;
        char ch, nl;
    
        head = NULL;
    
        for (i = 0; i < 21; i++) {
            enf[i] = ' ';
            enl[i] = ' ';
        }
    
        for (b = 0; b < 4; b++) {
            printf("Type in employee first name: ");
            gets(enf);
            printf("\nType in employee last name: ");
            gets(enl);
            printf("\nEnter user three digit ID: ");
            scanf("%d", & empID);
            printf("\n");
            head = insert (head, enf, enl, empID);
    
            for (i = 0; i < 21; i++)
            enf[i] = ' ';
            enl[i] = ' ';
        }
        p = head;
        while (p != NULL) {
            puts(head - > fnp);
            puts(head - > lnp);
            printf("The ID number is: ", head - > id);
            p = p - > next;
        }
    
        printf("Type any key to continue");
        scanf("%c", & ch);
    
        return 0;
    }
    
    job * insert (job *first, char empFNP[], char empLNP[], int idnum)
     {
        job *p, *q, *newp;
        int found,
        len,
        i;
        found = 0;
        q = first;
        p = first;
        char lnp[25];
        while ((p != NULL) && (!found)) {
            if (p - > id < idnum) {
                q = p;
                p = p - > next;
            } else found = 1;
        }
    
        newp = (job*) malloc(sizeof(job));
    
        newp - > id = idnum;
    
        strncpy(newp - > fnp, lnp, 21);
    
        newp - > next = p;
    
        if (q != p)
        q - > next = newp;
        else
        first = newp;
        return (first);
    Last edited by randyjohnson; 12-10-2012 at 07:18 PM.

  8. #8
    Registered User
    Join Date
    Oct 2012
    Posts
    23
    Come on guys, I'm really desperate here I just need to get this done so I can pass the semester. I'm getting to the point where I'll give 10 bucks via paypal to anyone who can get this running. I've been working on it all day and night and am making no progress.

  9. #9
    Lurking whiteflags's Avatar
    Join Date
    Apr 2006
    Location
    United States
    Posts
    9,613
    Quote Originally Posted by randyjohnson View Post
    Come on guys, I'm really desperate here I just need to get this done so I can pass the semester. I'm getting to the point where I'll give 10 bucks via paypal to anyone who can get this running. I've been working on it all day and night and am making no progress.
    Man, yopu basically don't listen to me anyway. I gave you a new insert function to write instead of the one that does not work, what more do you want? The one that you have doesn't do it correctly. And please keep your money: have some respect for yourself.

  10. #10
    Registered User
    Join Date
    May 2012
    Posts
    1,066
    Quote Originally Posted by randyjohnson View Post
    Come on guys, I'm really desperate here I just need to get this done so I can pass the semester.
    Code:
    gets(enf);
    FAQ > Why gets() is bad / Buffer Overflows - Cprogramming.com

    IMHO, anybody who still uses gets() shouldn't pass an exam.

    Bye, Andreas

  11. #11
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,318
    Line 42 isn't inside the loop because you didn't include braces.

    You've switched to using "gets". That is probably the worst change you could have made. Never ever ever use that function and under any circumstances . It is fundamentally flawed, can't be fixed, and can never result in a bug-free program. Use scanf or fgets.

    No matter what you say, the code as shown here can not compile because you have a space between the minus sign and the greater-than sign in every place that you mean to use the arrow operator.

    Always tell us what is going wrong. We don't tend to run your code for you.
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  12. #12
    Registered User
    Join Date
    Oct 2012
    Posts
    23
    Quote Originally Posted by AndiPersti View Post
    Code:
    gets(enf);
    FAQ > Why gets() is bad / Buffer Overflows - Cprogramming.com

    IMHO, anybody who still uses gets() shouldn't pass an exam.

    Bye, Andreas
    I've only been programming for 3 months. Excuse me for not knowing everything, I do what my professor teaches me. If that's a bad method, then oh well. But thank you for being so upstanding.

  13. #13
    Registered User hk_mp5kpdw's Avatar
    Join Date
    Jan 2002
    Location
    Northern Virginia/Washington DC Metropolitan Area
    Posts
    3,817
    #1
    Code:
    for (i = 0; i < 21; i++) {
        enf[i] = ' ';
        enl[i] = ' ';
    }
    
    ...
    
    for (i = 0; i < 21; i++)
        enf[i] = ' ';
        enl[i] = ' ';
    These don't do anything meaningful. You should probably do without these. The second one as mentioned by iMalc does not look correct because of the missing braces.





    #2
    Code:
    char lnp[25];
    
    ...
    
    newp = (job*) malloc(sizeof(job));
     
    newp - > id = idnum;
     
    strncpy(newp - > fnp, lnp, 21);
    As mentioned by iMalc, to access a member using a pointer you need "->" and not "- >" there should not be any space between the "-" and the ">" characters. Your posted code does this in at least 4 other places.

    Also, the local variable lnp is garbage when you attempt to copy data from it into newp->fnp since you haven't initialized it with anything. I think you mean to copy empFNP into newp->fnp and also copy empLNP into newp->lnp. Why do one and not the other?

    It is also advised not to cast the return value from the malloc call. The malloc function returns a void pointer type that can safely assigned to your job pointer type without the need of a cast.




    #3
    Code:
    printf("Type any key to continue");
    scanf("%c", & ch);
    This kind of thing often fails because of leftover data in the input buffer, typically a leftover newline character. A suggestion is to put a space in front of the %c character in the scanf function call like so:
    Code:
    scanf(" %c", &ch);
    The difference is subtle but important. The space will cause the scanf to skip over any leftover white-space that might be in the input buffer.





    #4 As mentioned/hinted, you should be using fgets instead of gets to read the names into your buffers but then you also need to deal with the potential newline character that might get stored along with the name.





    #5
    Code:
    printf("The ID number is: ", head - > id);
    Besides the "- >" issue already mentioned, you are missing a format specifier (%d) in this printf call if it is truly your intention to print an integer value out.





    #6 General comment: You look to allow the user to safely enter names up to 20 characters long but your struct allows storage up to 25.
    "Owners of dogs will have noticed that, if you provide them with food and water and shelter and affection, they will think you are god. Whereas owners of cats are compelled to realize that, if you provide them with food and water and shelter and affection, they draw the conclusion that they are gods."
    -Christopher Hitchens

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. C Linked Lists and Structures help!
    By Yorae in forum C Programming
    Replies: 0
    Last Post: 03-07-2006, 09:55 AM
  2. Can i do this: Structures and linked lists help
    By satory in forum C Programming
    Replies: 4
    Last Post: 04-25-2005, 07:49 AM
  3. help on linked lists(structures)
    By dionys in forum C Programming
    Replies: 3
    Last Post: 06-01-2004, 08:57 AM
  4. structures/linked lists
    By xddxogm3 in forum C++ Programming
    Replies: 4
    Last Post: 09-27-2003, 11:06 PM
  5. C Structures and Linked Lists.
    By bobthebuilder20 in forum C Programming
    Replies: 14
    Last Post: 04-15-2003, 05:00 PM