1

I am doing a small project for college (1st semester doing a bookstore implementation) and I have a problem with reading from a text file into a list of structs, with a two-dimensional array of characters in it that stores authors. However, it doesn't work properly (every time I launch the program it shows list is empty). Writing to a file works (I think, because it overwrites my text file with empty data).

Example data:

Adam Mickiewicz///Pan Tadeusz/Publisher 1/1833/24.99 Jules Verne///Around The World in 80 days/Publisher 1/1904/19.99 Jean-Jacques Sempe/Rene Goscinny//Little Nicholas/Publisher 2/1963/22.99 

My structure:

#define AK 3 // Constant denoting max number of authors typedef struct { char authors[AK][100]; char title[255]; char publisher[100]; unsigned int year; double price; struct Book *next; } Book; Book *first; // Address of the first element in a list 

Reading from file:

void read_from_file(const char *path) { int no_of_authors; int i; printf("Loading...\n"); FILE *fp = fopen(path, "r"); // Opening a file // Checking for errors if (!fp) { printf("Error reading from file!"); return; } // The loading mechanism no_of_authors = 0; while (!feof(fp)) { Book *new = (Book*) malloc(sizeof(Book)); for (i = 0; i < AK; i++) { fscanf(fp, "%s/", new->authors[i]); } fscanf(fp, "%s/%s/%u/%lf", new->title, new->publisher, &new->year, &new->price); fscanf(fp, "\n"); new = new->next; } fclose(fp); printf("Loading successful."); } 

Writing to file (just in case):

void write_to_file(const char *path, Book *first) { int i; printf("Saving...\n"); FILE *fp = fopen(path, "w"); Book* current = first; if (!fp) { printf("Error opening the file!"); dump_list(first); // Dumping the list to prevent memory leaks, this works okay } // Saving mechanism while (first != NULL) { for (i = 0; i < AK; i++) { fprintf(fp, "%s/", current->authors[i]); } fprintf(fp, "%s/%s/%u/%lf", current->title, current->publisher, &current->year, &current->price); fprintf(fp, "\n"); } fclose(fp); printf("Saved successfully"); } 
21
  • 3
    Thing get easier if you count cents in integer for price, instead of dollars in a double (or similar currency and subcurrency). Commented Dec 31, 2017 at 13:27
  • 1
    new = new->next; : where do you give a value to new->next ? Commented Dec 31, 2017 at 13:28
  • 4
    struct Book {}; is different from struct {} Book; Commented Dec 31, 2017 at 13:34
  • 4
    while (!feof(fp)) is wrong: it tells you the last read did not fail but does not tell you the next read will not fail. Commented Dec 31, 2017 at 14:02
  • 2
    typedef struct {...} Book; defines anonymous struct type, and typedefs it to type name Book. struct Book *next; defines an opaque pointer to type struct Book, which you never define (but this is ok, because it is a pointer and doesn't need to know the size). In general, avoid typedef with structs, because while it saves some typing, it also creates confusion (like demonstrated here). Commented Dec 31, 2017 at 15:42

1 Answer 1

3

OP's biggest failing is not checking the return value of fscanf(). Had code done so, problems would be more rapidly detected.


When is comes to reading lines of data the first consideration is:

Could input be faulty?

With learner applications this is often considered no. Input is either "good" or end-of-file. Let us not assume data is too well formated.

As it turns out, while the data file may not be faulty, the code reading it may be wrong. The subtle 2nd reason for code to check the *scanf() return values - self validation.


For line orientated data, it is much better to read is a line of data with fgets() than feof(), fscanf()... See also @Paul Ogilvie

 char buf[sizeof(Book) * 2]; // Use an ample sized buffer while (fgets(buf, sizeof buf, fp)) { 

Use "%s" to read in text that does not include white-space. This will also read in '/'. Since '/' is use to delimit, "%s" is not an acceptable input specifier. Further names like "Adam Mickiewicz" include a space. A 2nd reason to not use "%s".

Consider what fscanf(fp, "%s/", new->authors[i]); is doing. "%s" scans into new->authors[i] non-white-space characters. A character after non-white-space characters is a white-space, never a '/'.

Use "%[^/]" to read in text that does not include '/'.

Use "%n" to keep track of the current scan offset.

Now parse the line.

 char *p = buf; Book *nu = malloc(sizeof *nu); // no cast needed for (size_t i = 0; i < AK; i++) { int n = 0; sscanf(p, "%99[^/]/%n", nu->authors[i], &n); if (n == 0) { Handle_Parse_Error(); } p += n; } if (sscanf(p, "%254[^/]/%99[^/]/%u/%lf", nu->title, nu->publisher, &nu->year, &nu->price) != 4) { Handle_Parse_Error(); } 

Robust code would add checks on each member. Suit to coding goals.

 if (nu->year < -6000 || nu->year > 2999) Fail_Year_Range(); 

Further work is needed to link data together. OP's code is unclear on this matter and so left to OP. A possible approach:

Book *read_from_file(const char *path) { Book first; // Only the .next member is used. first.next = NULL; Book *current = &first; // setup while () loop ... while (fgets(buf, sizeof bu, fp)) { ... Book *nu = malloc(sizeof *nu); nu->next = NULL; ... // parse data, fill in rest of nu-> .... current->next = nu; // update current pointer current = nu; } // close up file ... return first.next; } 
Sign up to request clarification or add additional context in comments.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.