-1

So I have made a working program for getting data from a file and printing out the file according to which part of the file you want all within main. But my goal is to make this modular, by creating a user interface and either appending to the linked list from the file or printing out a part of that linked list (if any) as requested. There's just one problem: I can't seem to figure out a way to successfully pass a linked list to the function so that when you create new nodes in the function (Append), it will also work in main and then back again in (Print).

Here's the working code:

#include <stdio.h>
#include <stdlib.h>      /* has the malloc prototype      */
#define TSIZE 100        /* size of array to hold title   */
struct book {
    char title[TSIZE], author[TSIZE],year[6];
    struct book * next;  /* points to next struct in list */
};
//typedef struct book ITEM;
typedef struct book * Node;     // struct book *
void Append(Node *List, Node *Lcurrent,char filename[]);
void ClearGarbage(void);
int main(void){
    Node head=NULL;
    Node current;
    current=head;
    char fname[]="HW15Data.txt";
    char op;
    do{
        puts("Select operation from the following list:");
        puts("a. Append    p. Print q. quit");
        op = getchar();
        ClearGarbage();
        if (op=='a'){
            /* Gather  and store information          */
            Append(&head,&current,fname);
        }
        else if (op=='q'){
            /* User quit, so free allocated memory */
            current = head;
            while (current != NULL)
            {
                free(current);
                current = current->next;
            }
            printf("Bye!\n");
            return 0;
        }
        else{
            /* Program done, so free allocated memory */
            current = head;
            while (current != NULL)
            {
            free(current);
            current = current->next;
            }
            printf("Invalid characted entered. Bye!\n");
            return 0;
        }
    } while (op!='q');
    return 0;
}
void Append(Node *List, Node * Lcurrent,char filename[TSIZE]){
    FILE * fp;
    Node head=*List;
    Node current=*Lcurrent;
    int loops=0;
    fp=fopen(filename,"r");
    if (head==NULL){
        head=(Node) malloc(sizeof(struct book));
        current=head;
        current->next=NULL;
    }
    do{
        current->next = (Node) malloc(sizeof(struct book));
        current=current->next;
        loops++;
    } while(fgets(current->title,sizeof(current->title),fp) && fgets(current->author,sizeof(current->title),fp) && fgets(current->year,sizeof(current->year),fp));;
    free(current);
    printf("Number of records written: %d\n",loops);
//Same as Print function in the nonworking code
    int num;
    int i=0;
    if (head == NULL){
        printf("No data entered. ");
    }
    else{
        printf("Enter record # to print: ");
        scanf("%d",&num);
        ClearGarbage();
        num=num+1;
        current = head;
        while (current != NULL && i<num)
        {
            for (i=0;i<num;i++)
                current = current->next;
            printf("Book: %sAuthor: %sYear: %s\n",
            current->title, current->author, current->year);
            }
    }
}
void ClearGarbage(void){
    while (getchar()!='\n');
}

Okay cool that works, but my guess is that as soon as Append is done, the nodes made in Append are useless in main because they are now gone. So when I try to make a Print function in the following code, there's nothing to print.

#include <stdio.h>
#include <stdlib.h>      /* has the malloc prototype      */
#define TSIZE 100        /* size of array to hold title   */
struct book {
    char title[TSIZE], author[TSIZE],year[6];
    struct book * next;  /* points to next struct in list */
};
//typedef struct book ITEM;
typedef struct book * Node;     // struct book *
void Append(Node *List, Node *Lcurrent,char filename[]);
int Print(Node *List,Node *Lcurrent);
void ClearGarbage(void);
int main(void){
    Node head=NULL;
    Node current;
    current=head;
    char fname[]="HW15Data.txt";
    char op;
    do{
        puts("Select operation from the following list:");
        puts("a. Append    p. Print q. quit");
        op = getchar();
        ClearGarbage();
        if (op=='a'){
            /* Gather  and store information          */
            Append(&head,&current,fname);
        }
        else if (op=='p'){
            /*Print book record of user's choice*/
            Print(&head,&current);
        }
        else if (op=='q'){
            /* User quit, so free allocated memory */
            current = head;
            while (current != NULL)
            {
                free(current);
            current = current->next;
            }
            printf("Bye!\n");
            return 0;
        }
        else{
            /* Program done, so free allocated memory */
            current = head;
            while (current != NULL)
            {
            free(current);
            current = current->next;
            }
            printf("Invalid characted entered. Bye!\n");
            return 0;
        }
    } while (op!='q');
    return 0;
}
void Append(Node *List, Node * Lcurrent,char filename[TSIZE]){
    FILE * fp;
    Node head=*List;
    Node current=*Lcurrent;
    int loops=0;
    fp=fopen(filename,"r");
    if (head==NULL){
        head=(Node) malloc(sizeof(struct book));
        current=head;
        current->next=NULL;
    }
    do{
        current->next = (Node) malloc(sizeof(struct book));
        current=current->next;
        loops++;
    } while(fgets(current->title,sizeof(current->title),fp) && fgets(current->author,sizeof(current->title),fp) && fgets(current->year,sizeof(current->year),fp));
    free(current);
    printf("Number of records written: %d\n",loops);
}
int Print(Node *List,Node *Lcurrent){
    int num;
    int i=0;
    Node head=*List;
    Node current=*Lcurrent;
    if (head == NULL){
        printf("No data entered.\n");
        return -1;
    }
    printf("Enter record # to print: ");
    scanf("%d",&num);
    ClearGarbage();
    num=num+1;
    current = head;
    while (current != NULL && i<num)
    {
        for (i=0;i<num;i++){
            current = current->next;
        }
        printf("Book: %sAuthor: %sYear: %s\n",
        current->title, current->author, current->year);
        }
    return 0;
}
void ClearGarbage(void){
    while (getchar()!='\n');
}

Thanks anyone for any help!

EDIT: Got rid of an unused typedef for clarity

King Sutter
  • 61
  • 2
  • 4
  • 11
  • 1
    Hmmm, type-defining a point like `typedef struct book * Node;` is un-favored practice amongst some coding styles. – chux - Reinstate Monica May 03 '17 at 19:17
  • `typedef struct Node List;` is this even valid? – Ajay Brahmakshatriya May 03 '17 at 19:18
  • @AjayBrahmakshatriya Sure, `typedef struct Node List;` compiles. – chux - Reinstate Monica May 03 '17 at 19:19
  • @chux But isn't Node the same as `struct book *`. What is `struct Node`? Unless `struct Node` is some completely other struct? – Ajay Brahmakshatriya May 03 '17 at 19:20
  • 1
    @chux I think `typedef struct Node List` is creating an incomplete type `struct Node` and typedefing it to List. It has nothing to do with the Node in the previous line. – Ajay Brahmakshatriya May 03 '17 at 19:24
  • @AjayBrahmakshatriya `Node` is the same as `struct book *` as per your comment. `struct Node` is not defined yet. Agree it has nothing to do with `Node` in the previous line yet your question was `typedef struct Node List;` is this even valid? – chux - Reinstate Monica May 03 '17 at 19:24
  • 1
    Anyway `List` type is not used anywhere. It is just creating confusion. – Ajay Brahmakshatriya May 03 '17 at 19:26
  • `fgets(current->title, sizeof(line), fp) && fgets(current->author, sizeof(line), fp) && fgets(current->year, sizeof(line), fp));` is bad code. Should use sizeof target buffer, not `sizeof line`. – chux - Reinstate Monica May 03 '17 at 19:30
  • I forgot to get rid of typedef struct Node List. It's useless. Sorry for the confusion! – King Sutter May 03 '17 at 19:31
  • @KingSutter Consider [Is it a good idea to typedef pointers?](http://stackoverflow.com/q/750178/2410359) – chux - Reinstate Monica May 03 '17 at 19:34
  • 1
    @chux You are right, I got confused by the `while` and `for` afterwards. – mch May 03 '17 at 19:34
  • I was considering not using typedef, myself, but I thought it would make my code more readable/easier to write but I'm actually second guessing that now, @chux . Either way, I don't see that as being a way to solve the issue of not being able to pass a linked list from function to function. Honestly I think the more I try to, the more I get confused. :-/ – King Sutter May 03 '17 at 19:41
  • Is there any hope for this code? I'm getting the vibe that I'm using bad coding practices all around here. I might have to draw my plan out and just start from scratch then – King Sutter May 03 '17 at 19:43
  • @KingSutter The more difficult code is to read, the more difficult it is to debug. Consider a recipe for cooking with many mis-spelled words. The spelling does not _directly_ affect the meal, but it adds unneeded challenges. Concerning this style issue: Best to code per your group's coding standard. – chux - Reinstate Monica May 03 '17 at 19:44
  • @king Did you try fixing your code per this [comment](http://stackoverflow.com/questions/43768210/how-to-pass-a-linked-list-to-a-function-in-c?noredirect=1#comment74578412_43768210)? – chux - Reinstate Monica May 03 '17 at 19:45
  • What if I passed the linked list from function to function by having the functions return Node or something like that? That way I can define BookList or something and set to equal that function. And then when I want to use Print, I just pass BookList to it. – King Sutter May 03 '17 at 19:45
  • @chux I'll see what I can do, I actually had that messy while statement in a different format before posting my code here, but my professor suggested earlier today that I use the while statement that way for some reason. Wait, where is target buffer defined? – King Sutter May 03 '17 at 19:48
  • @KingSutter the problem is not the `while`. The problem is `sizeof(line)` is the wrong size. – chux - Reinstate Monica May 03 '17 at 19:49
  • @chux the target buffer is honestly undefined to an extent. I see what you are getting at, but I don't know the exact amount of characters of what fget will read per line. I thought fgets reads until it encounters a new line, which is what I want it to do. The maximum characters to read shouldn't matter as long as it isn't set too low. That being said, I will get rid of the sizeof() part and change it to TSIZE. Which is the max size I set the title and author character arrays in the book structure to be anyways. – King Sutter May 03 '17 at 20:03
  • @KingSutter `TSIZE` is the wrong value too. `fgets(current->year,sizeof(line),fp)` has only room for 6 characters. Use `fgets(current->year, sizeof current->year, fp)` Like-wise for the other `fgets()`. `fgets()` reads until 1) `'\n'` read 2) buffer full or 3) end-of-file or 4) rare input error. – chux - Reinstate Monica May 03 '17 at 20:04
  • @chux I changed that for the year one, too. It is 6. But i can change it again per your suggestion, as well. That makes sense, thank you for elaborating :) I'm slow today, I guess – King Sutter May 03 '17 at 20:08

1 Answers1

1

It seems like most of the people are focusing the lack of organization (it bother me a lot too) instead of your actual issue.

Seems like the source of your issue is where you assign the variable head. when you define "Node head = *List" and List is NULL, as it's first initialized, it loses the connection it had to the original list you sent from main, and you just create a linked list with a local reference only.

I just changed the uses of "head" to "*List" within the Append and Print functions and it seems to sort it out.

This is your code after my changes:

#include <stdio.h>
#include <stdlib.h>      /* has the malloc prototype      */
#define TSIZE 100        /* size of array to hold title   */
struct book {
    char title[TSIZE], author[TSIZE], year[6];
    struct book * next;  /* points to next struct in list */
};
//typedef struct book ITEM;
typedef struct book * Node;     // struct book *
void Append(Node *List, Node *Lcurrent, char filename[]);
int Print(Node *List, Node *Lcurrent);
void ClearGarbage(void);
int main(void) {
    Node head = NULL;
    Node current;
    current = head;
    char fname[] = "HW15Data.txt";
    char op;
    do {
        puts("Select operation from the following list:");
        puts("a. Append    p. Print q. quit");
        op = getchar();
        ClearGarbage();
        if (op == 'a') {
            /* Gather  and store information          */
            Append(&head, &current, fname);
        }
        else if (op == 'p') {
            /*Print book record of user's choice*/
            Print(&head, &current);
        }
        else if (op == 'q') {
            /* User quit, so free allocated memory */
            current = head;
            while (current != NULL)
            {
                free(current);
                current = current->next;
            }
            printf("Bye!\n");
            return 0;
        }
        else {
            /* Program done, so free allocated memory */
            current = head;
            while (current != NULL)
            {
                free(current);
                current = current->next;
            }
            printf("Invalid characted entered. Bye!\n");
            return 0;
        }
    } while (op != 'q');
    return 0;
}
void Append(Node *List, Node * Lcurrent, char filename[TSIZE]) {
    FILE * fp;
    Node head = *List;
    Node current = *Lcurrent;
    int loops = 0;
    char line[256];
    fp = fopen(filename, "r");
    if (*List == NULL) {
        *List = (Node)malloc(sizeof(struct book));
        current = *List;
        current->next = NULL;
    }
    do {
        current->next = (Node)malloc(sizeof(struct book));
        current = current->next;
        loops++;
    } while (fgets(current->title, sizeof(line), fp) && fgets(current->author, sizeof(line), fp) && fgets(current->year, sizeof(line), fp));
    free(current);
    printf("Number of records written: %d\n", loops);
}
int Print(Node *List, Node *Lcurrent) {
    int num;
    int i = 0;
    Node head = *List;
    Node current = *Lcurrent;
    if (*List == NULL) {
        printf("No data entered.\n");
        return -1;
    }
    printf("Enter record # to print: ");
    scanf("%d", &num);
    ClearGarbage();
    num = num + 1;
    current = *List;
    while (current != NULL && i<num)
    {
        for (i = 0; i<num; i++) {
            current = current->next;
        }
        printf("Book: %sAuthor: %sYear: %s\n",
            current->title, current->author, current->year);
    }
    return 0;
}
void ClearGarbage(void) {
    while (getchar() != '\n');
}

There are still many logical errors, and some bugs, but it fixes the problem you asked help for.

NK_loco
  • 84
  • 7
  • This answer does not take into account `fgets(current->title, sizeof(line),...` as commented [here](http://stackoverflow.com/questions/43768210/how-to-pass-a-linked-list-to-a-function-in-c?noredirect=1#comment74578412_43768210) and so continues potential undefined behavior (UB). – chux - Reinstate Monica May 03 '17 at 19:51
  • I said there were many other errors and bugs, but he asked about the problem of him sending the linked list. – NK_loco May 03 '17 at 19:52
  • `*List = (Node)malloc(sizeof(struct book));` is a good important improvement over OP's code. – chux - Reinstate Monica May 03 '17 at 20:12
  • That did it! Thank you, @NK_loco and chux! I am currently doing a lot of fixes to make it more readable and bug fixes. I just couldn't see a way past the function passing issue so I mainly needed that cleared up. I apologize if that was a mouthful of crappy code to look through, so thank you for bearing with me on that one. – King Sutter May 03 '17 at 20:32
  • 1
    np ^.^ Pointers could be tricky at times, just make sure you understand what points to what and what assigns to what. changing things localy with pointers is very common, but you'll get it with enough practice ;) – NK_loco May 03 '17 at 20:37
  • One quick question, would it be important to also replace the "current" variables with "*Lcurrent" inside of each function? Or does it not matter? – King Sutter May 03 '17 at 20:39
  • Yeah pointers lose me quickly and then learning about these linked lists on top of having to make it modular melted my brain haha – King Sutter May 03 '17 at 20:40