0

So I have this code that reads a bunch of lines from a csv document. I know that initially the document has 16 rows, and that's why I designated int noRows = 16; in my main funtion. 

void readBeer(int noRows) {
char *oneline, *token;
char oneproduct[256];
char delim[] = ",";
int x = 1;

FILE *fp; //open file
if ((fp = fopen("varor.csv", "r")) == NULL) //can the file be opened?
{
fprintf(stderr, "File varor.csv couldn't be opened\n"); //"couldn't open file"
exit(-1);
}

while(noRows != 0) 
{
    int countTok = 1;

    fgets(oneproduct, 256, fp); //get the first row
    oneproduct[strlen(oneproduct) - 1] = '\0'; // remove end-of-line character

    oneline = strdup(oneproduct); //duplicate oneproduct into oneline because strtok modifies the given string
    token = strtok(oneline, delim); //split oneline into tokens, tokens are separated by ","

    while (token != NULL) 
    {
        if(countTok == 1) beer[x].productNumber = atoi(token);
        else if(countTok == 2) strcpy(beer[x].name, token);
        else if(countTok == 3) beer[x].price = atof(token);
        else if(countTok == 4) beer[x].volume = atof(token);
        else if(countTok == 5) strcpy(beer[x].type, token);
        else if(countTok == 6) strcpy(beer[x].style, token);
        else if(countTok == 7) strcpy(beer[x].packaging, token);
        else if(countTok == 8) strcpy(beer[x].country, token);
        else if(countTok == 9) strcpy(beer[x].manufacturer, token);
        else if(countTok == 10) beer[x].alcohol = atof(token);
        else printf("kossan hoppade!"); //should never be seen in console
        token = strtok(NULL, delim);
        countTok++;
    }

    x++;
    noRows--;
    free(oneline); free(token);

}

fclose(fp);

}

My question is how do I read the file to its end without first knowing how many rows it has? I'm thinking of having a specific cell in the file just to save noRows between startup and shutdown of console.

I tried using char buffer[1000]; while(fgets(buffer, 1000, fp)) {} but then it reads the first 8 rows(not sure if it's always exactly 8) as 0,0,0,0,0,0,0,0,0,0.

David Hermansson
  • 23
  • 2
  • 3
    Did you try to check whether you reached EOF? BTW: Are you sure that the last line holding values contains a `\n` at the end? – Gerhardh Jan 12 '18 at 11:40
  • 3
    Please, use switch(), so many else if-s look terrible. – Gnqz Jan 12 '18 at 11:42
  • When I used the buffer, it stopped after the last line, so it registered the EOF, yeah. What does the \n matter on the last line? – David Hermansson Jan 12 '18 at 11:43
  • @Gnqz I usually want it to work before I make it look good, but I'll edit it. – David Hermansson Jan 12 '18 at 11:45
  • You always remove the last character of a read line. If the last line does not end with `\n`, you remove one character of your "payload". Text files where last line does not end with `\n` are quite usual (i.e. may occur more often as you imagine as you usually cannot see this in the text editors). – Scheff's Cat Jan 12 '18 at 11:48
  • 1
    Regarding your actual question: Your attempt to re-design is promising. Try [`getline()`](http://man7.org/linux/man-pages/man3/getline.3.html) instead of `fgets()`. it returns `-1` if it fails. – Scheff's Cat Jan 12 '18 at 11:53
  • The `free(token)` is IMHO wrong. It probably has no negative effect as it is always left with `NULL` after a row has been processed. `strtok()` does not allocate new storage. Instead, it modifies the original string (which is passed at first call). – Scheff's Cat Jan 12 '18 at 11:59
  • Concerning `strtok()`: I once posted a variation of `strtok()` as answer to [SO: Split string into Tokens in C, when there are 2 delimiters in a row](https://stackoverflow.com/a/42315689/7478597). It works quite similar like `strtok()` and may illustrate how "they" probably do it internally. – Scheff's Cat Jan 12 '18 at 12:03

2 Answers2

1

The answer to your question if just test the return value of fgets

for (;;)   // idiomatic C style for an infinite loop 
{
    int countTok = 1;

    if (NULL == fgets(oneproduct, 256, fp)) break; //get one row and exit loop on EOF
    ...

But as you were told in comments, there are many other problems in your code:

  • you initialize your indexes to 1 while C array indexes start at 0. So I assume that x=1; should be x=0;. For the same reason, countTok = 1; is not wrong, but countTok = 0; would be more idiomatic C
  • you use fprintf(stderr, ... to notice an error when opening the file. While not wrong, it gives no indication on the cause of the error. perror would do...

  • you erase last character of oneproduct without controling it is a newline. The assumption will be wrong

    • if one line contains at least 256 characters
    • if last line does not end with a newline

    The idiomatic way is to use strcspn:

    oneproduct[strcspn(oneproduct, "\n")] = '\0';  // erase an optional end of line
  • you duplicate oneproduct to oneline. Here again, nothing is wrong, but it is useless because you never use the original line
  • the long list of else if could be replaced with a switch, but this one is mainly a matter of style
  • you free token at the end of loop. As it is NULL it is a no-op, but it is was not allocated it should not be freed.
Serge Ballesta
  • 121,548
  • 10
  • 94
  • 199
0

If you're using fgets, you can test if the result of the call is equal to NULL or not.fgets signals NULL on error or EOF (end-of-file).

char *fgets(char *line, int maxline, FILE *fp);.

I.E:

while (fgets(buffer, MAXLINE, fp) != NULL) {
    // Process line here.
}

You can also process the entire file character by character and test if (c == EOF). There are a couple of ways to do it.

Micrified
  • 2,632
  • 3
  • 30
  • 45