3

I'm trying to build a simple function that will take in a data file, and assign various values from the data file into a global array of structures. However, I'm having trouble getting it to work quite right. I've written what I believe is most of the needed code, but my test line printf("time is %d\n", BP[i].time); simply reads out "Time is 0." 10 times, leading me to believe the values aren't getting assigned to the structure array like I imagined they would be.

How can I proceed further?

Example Data File (.txt):

0001    553    200
0002    552    100
....    ...   ...

Current Code:

#include <stdio.h>
#include <stdlib.h>

// Function Prototype
void readFileBP(char fileName[1000]);

// Definition of BP Structure
struct bloodPressure
{
    int *time;
    int *sys;
    int *dia;
}BP[50]; // end struct BP

int main()
{
    char fileName[1000] = "C:\\Users\\User\\Desktop\\DataFiles\\BP_1.txt";
    readFileBP(fileName);

    int i = 0;

    for (i; i<10; i++)
    {
        printf("Time is %d\n", BP[i].time);
    }
} // end int main()

void readFileBP(char fileName[1000])
{
    FILE *filePtr; // declare file pointer
    int time;
    int sys;
    int dia;
    int position = 0;


    if (filePtr = fopen(fileName, "r") == NULL) // error check opening file
    {
        printf("Opening file failed. Please reenter filename.");
        exit(1); 
    } // end if

    while (fscanf(filePtr, "%d, %d, %d", &time, &sys, &dia) != EOF) // read in BP values
    {
        BP[position].time = time;
        BP[position].sys = sys;
        BP[position].dia = dia;
        position++;

    } // end while

    fclose(filePtr);



} // end void readFile()
gsamaras
  • 71,951
  • 46
  • 188
  • 305
MomoDevi
  • 63
  • 1
  • 6
  • This now is the perfect opportunity to learn of to use a debugger. Step through the code inspecting all relevant variables to see what is *really* going on! :-) – alk Apr 18 '16 at 12:41
  • 3
    Look at this `(filePtr = fopen(fileName, "r") == NULL)` twice. What happens 1st here? – alk Apr 18 '16 at 12:43
  • And as a minimal debugging support you want to print out the values read *inside* the loop. Does anything gets read at all? – alk Apr 18 '16 at 12:45
  • @alk Thanks for your help identifying the offending line. I really should've read out the values within the loop, as you said. I'm a beginner, so I don't understand most of the warnings my compiler throws at me (such as it `expecting an argument of type int but argument has type int *`), but I managed to see that the wrong thing was probably getting assigned to filePtr. So instead, I moved the opening to a line above the if statement. It's a little less cleaner, but I think gets the same job done! – MomoDevi Apr 18 '16 at 14:12

3 Answers3

1

Compile with warnings enabled. You should get something like that:

gsamaras@gsamaras-A15:~$ gcc -Wall -o px px.c
px.c: In function ‘main’:
px.c:22:5: warning: statement with no effect [-Wunused-value]
     for (i; i<10; i++)
     ^
px.c:24:9: warning: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘int *’ [-Wformat=]
         printf("Time is %d\n", BP[i].time);
         ^
px.c: In function ‘readFileBP’:
px.c:37:17: warning: assignment makes pointer from integer without a cast [enabled by default]
     if (filePtr = fopen(fileName, "r") == NULL) // error check opening file
                 ^
px.c:37:5: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
     if (filePtr = fopen(fileName, "r") == NULL) // error check opening file
     ^
px.c:45:27: warning: assignment makes pointer from integer without a cast [enabled by default]
         BP[position].time = time;
                           ^
px.c:46:26: warning: assignment makes pointer from integer without a cast [enabled by default]
         BP[position].sys = sys;
                          ^
px.c:47:26: warning: assignment makes pointer from integer without a cast [enabled by default]
         BP[position].dia = dia;
                          ^
px.c: In function ‘main’:
px.c:26:1: warning: control reaches end of non-void function [-Wreturn-type]
 } // end int main()
 ^

Isn't that enough to get you started? It was for me! :)

gsamaras
  • 71,951
  • 46
  • 188
  • 305
  • 1
    This should have been a comment, but obviously is too long for one. So get a 1+. ;-) – alk Apr 18 '16 at 13:04
  • 1
    Lessons learned: Push up the compiler's warning level to the maximum and fix the code until no more warnings are issued. If still problems are around come back here ... – alk Apr 18 '16 at 13:05
  • 1
    @alk to be honest I show your comments and thought I should leave it to you. But then I thought that I could show what the compiler has to say about your comment and post that as a comment. Turns out it's a rather broad question, so I think that should trigger the debugging experience for OP, combined with gdb for maximum pleasure! :) – gsamaras Apr 18 '16 at 13:06
  • 1
    "*... for maximum pleasure!*" Great wording!-) – alk Apr 18 '16 at 13:08
1

I made some changes and ran it just now.

    #include <stdio.h>
    #include <stdlib.h>

// Function Prototype
void readFileBP(char fileName[1000]);

// Definition of BP Structure
struct bloodPressure
{
    int time;
    int sys;
    int dia;
}; // end struct BP
struct bloodPressure BP[50];

int main()
{
    char *fileName = "file.txt";
    readFileBP(fileName);
    int i = 0;
    for (i; i<10; i++)
    {
        printf("Time is %d\n", BP[i].time);
    }
    getch();
}

void readFileBP(char fileName[1000])
{
    FILE *filePtr; // declare file pointer
    int time=0;
    int sys=0;
    int dia=0;
    int position = 0;
    filePtr= fopen(fileName,"r");
    while (fscanf(filePtr, "%d, %d, %d", &time, &sys, &dia) != EOF) // read in BP values
    {
        BP[position].time = time;
        BP[position].sys = sys;
        BP[position].dia = dia;
        position++;

    } // end while

    fclose(filePtr);
} // end void readFile()

The output is now:

Time is 1
Time is 553
Time is 200
Time is 2
Time is 552
Time is 100
Time is 0
Time is 0
Time is 0
Time is 0
Arjun
  • 817
  • 3
  • 16
  • 28
  • You do not want to fix warnings by blindly casting around like here `BP[position].time = (int *)time;` Just correct the definitions of `struct bloodPressure`'s members to suite the correct type. Currently it defines all its members to be a pointer to `int`, what you do not seem to want. Just make them `int` by removing the star, so `int * time` becomes `int time`. – alk Apr 18 '16 at 14:30
  • @alk I was actually considering that when I first saw the code but then I thought they required it as `int * time`. But, thanks for the advice.. no more blind casting. – Arjun Apr 18 '16 at 15:15
  • @alk The answer has now been edited with `int time` instead of `int * time`. – Arjun Apr 18 '16 at 15:17
0

try changing the line :

while (fscanf(filePtr, "%d, %d, %d", &time, &sys, &dia) != EOF)

to

while (fscanf(filePtr, "%d%d%d", &time, &sys, &dia) != EOF)

also here's what i tried out and it seems to work based on the tests i've done

#include <stdio.h>
#include <stdlib.h>

#define MAX_ARRAY_SIZE 50

typedef struct BloodPressure
{
    int time;
    int sys;
    int dia;
}BloodPressure;

BloodPressure bloodPressure[MAX_ARRAY_SIZE];

void ReadFile(char *fileName);

int main(int argc, char *argv[])
{
    char *fileName = "BP_1.txt";

    ReadFile(fileName);

    int i = 0;

    for (i = 0; i < MAX_ARRAY_SIZE; i++)
    {
        printf("Dia is : %d\n", bloodPressure[i].dia);
        printf("Sys is : %d\n", bloodPressure[i].sys);
        printf("Time is : %d\n", bloodPressure[i].time);
        printf("\n");
    }

    exit(EXIT_SUCCESS);
}

void ReadFile(char *fileName)
{
    FILE *filePtr = NULL;
    int  i = 0;

    if ((filePtr = fopen(fileName, "r")) == NULL)
    {
        printf("Error : Unable to open %s for reading\n");
        exit(EXIT_FAILURE);
    }

    while (fscanf(filePtr, "%d%d%d", &bloodPressure[i].dia, &bloodPressure[i].sys, &bloodPressure[i].time) != EOF)
    {
        i++;
    }

    fclose(filePtr);
}
sk1984
  • 24
  • 3
  • Yeah -- I had a parentheses error, after all (in the if statement, not the while statement though). Your code: `if((filePtr = fopen(fileName, "r")) == NULL)` vs my code: `if (filePtr = fopen(fileName, "r") == NULL)` – MomoDevi Apr 18 '16 at 19:45
  • don't worry about it too much, that's happened to me a couple times too :D – sk1984 Apr 18 '16 at 20:37