0

I'm working on a project for my CS class, part of the required program is loading data from a comma-delimited .txt file into a struct array. Everything seems to be working as expected, but when I print the data from the struct array (with one line between each complete entry), it looks like there are empty entries?

The data in the file is in the following format: Last Name,First Name,Address,City,Phone\n

I used fgets() to get each file in a string, then divide it into tokens, each one is placed in its respective place of the struct.

Also, I thought the variable 'j' is supposed to indicate how many entries I had? Isn't that correct?

Here's my code:

#include "project.h"

void load(char path[])
{
  FILE * f;
  char line[300];
  f = fopen(path,"r");
  j=0;
  if (f != NULL)
    {
      while(!(feof(f)))
        {
          fgets(line,300,f);
          char *parts;
          parts=strtok(line, ",");
          int i=0;
          while (parts !=NULL)
            {
              switch (i)
                {
                case 0:
                  strcpy(entries[j].last, parts);
                  break;
                case 1:
                  strcpy(entries[j].first, parts);
                  break;
                case 2:
                  strcpy(entries[j].address, parts);
                  break;
                case 3:
                  strcpy(entries[j].city, parts);
                  break;
                case 4:
                  strcpy(entries[j].phone, parts);
                  break;
                }
              parts=strtok(NULL, ",");
              i++;
            }
          j++;
        }
      fclose(f);
      printf("Data has been loaded. Program will return to main menu in 3 seconds.\n");
      sortEntries();
    }
  else
    printf("\nERROR! FILE DOES NOT EXIST! The program will return to main menu in 3 seconds.\n");

  menu();
}

Here's my printing function: (Maybe the error's there)

void print()
{

    printf("FORMAT:\nLast Name,First Name,Address,City,Phone Number\n\n");
    for (int i=0; i<j; i++)
    {
       printf("%s,%s,%s,%s,%s\n", entries[i].last, entries[i].first, entries[i].address, entries[i].city, entries[i].phone);
    }
}

I tried running without the sortElements() function so I know if the error's in it, but it was still there.

The entries:

struct entry {
char last[50];
char first[50];
char address[140];
char city[50];
char phone[15];
};
struct entry entries[1000]; 
Mariam
  • 1
  • 1
  • 1
    The part you posted seems correct at first glance. But you don't show us the definition of entries - how are they defined? Do you allocate enough memory for them? What about the components (last, first, ...) - do you define them as character arrays, or pointers? And if pointers, do you allocate space for them? – Guntram Blohm Dec 12 '13 at 17:08
  • Debugging suggestion: add a `printf` after each `strcpy` to see what the program thinks it's copying. Add a `printf` after the `while` loop is complete to see what's in `entries[j]`. – Tim Pierce Dec 12 '13 at 17:13
  • Hm. What does `sortEntries()` do? Is it possible that's wiping out your data? – Tim Pierce Dec 12 '13 at 17:14
  • Or, show us an example of the output. You're using fgets() to read the lines, that function does NOT remove the trailing \n. So, your phone numbers will include the \n, which means that you have an empty line after each line you output (one \n from the phone and one \n from the format string). You can distinguish these from empty records - empty records should at least show the terminating commas. – Guntram Blohm Dec 12 '13 at 17:15
  • See [Why `while (!feof(file))` is always wrong](http://stackoverflow.com/questions/5431941/while-feof-file-is-always-wrong) to understand why you process one line too many. Consider what happens if there are commas missing. Note that `strtok()` accepts multiple adjacent delimiters as a single delimiter; this is not what you want if there can be empty fields in your data. And it means that your structure could be incompletely initialized; if there are only 2 fields, then `address`, `city`, `phone` won't be set. – Jonathan Leffler Dec 12 '13 at 17:31

1 Answers1

1

You're not stripping the newlines from the lines you read in - fgets doesn't remove them. So, your parts string will have a \n at the end. This \n will be in your phone string as well. So, your printf will print two newlines after each line, one from the phone string and one from the format string. Use strtok(NULL, ",\n") to fix this and make the line feed a terminator.

Also, you're using feof, which is evil, because it won't be set until AFTER you try to read something when there's no data. So after reading the last line, feof will not be set, you read another line with fgets (which fails because there's no data), then do your strtok stuff on this non-existing line. It's much better NOT to use feof, and check the return value of fgets, like this:

while (fgets(line, 300, f)!=NULL) {
    parts=strtok(line, ",");
    ....
    j++;
}

In Reality, using feof() is almost always wrong. However, instructors seem to like it way too much to ever teach that to students ;)

Guntram Blohm
  • 9,667
  • 2
  • 24
  • 31
  • I tried this, no more unnecessary spaces BUT I still get a line that's all ",,,,". Also, even though j is supposed to be = the number of entries, it only prints four lines (no matter how many entries there are) – Mariam Dec 12 '13 at 17:34
  • Did you change the feof() thing? And, did you make sure your input file doesn't contain an extra empty line at the end? Your program seems ok, but it would be helpful if you added a `printf("j=%d\n", j)` to the end of load() and to the start of print(), to make sure the variable is what it should be and doesn't get modified somewhere in between. – Guntram Blohm Dec 12 '13 at 17:40
  • Yes, I changed the feof() part. – Mariam Dec 12 '13 at 17:44
  • It says j=4 in both cases, I don't know why? The file currently has 8 lines. Also tried it with another file that has another number of entries. Also 4? – Mariam Dec 12 '13 at 17:46
  • Something weird must happen in the rest of your program, i can't explain the 4 from what you've posted. Put something like `printf("i=%d j=%d parts=%d\n")` in front of the switch, and compare the output with what you'd expect. Also, add the definition of entries to your post, maybe there's something wrong with that. – Guntram Blohm Dec 12 '13 at 17:58
  • I added the entries to the bottom of the post. I'll try to print values of i and j in front of the switch to see what happens. – Mariam Dec 12 '13 at 18:02