Data Input/Output 
Author Message
 Data Input/Output

Could someone please tell me why the following program doesn't work? I
have read through it several times and cant see anything obviously wrong
with it.

Thanks,
Jamie Fraser

------

#include <stdio.h>
#include "c:\My Documents\Ac1101\Projects\include\genio.h"

#define NUMBEROFJUDGES 5
#define GYMNASTS 15

typedef struct
{
        char Name[40], Country[30];
        int Age;
        float JudgeScores[NUMBEROFJUDGES];
        float FinalScore;

Quote:
} Gymnast;

Gymnast CalculateScore(FILE *DataFile);
void StoreData(Gymnast);

void main(void)
{
        char FileName[50];
        FILE *DataFile;
        int i,x;
        Gymnast Result[GYMNASTS];

        puts ("Please enter the name of the file containing Gymnast
Data:");
        gets (FileName);

        DataFile = fopen(FileName, "r");

        if((DataFile = fopen(FileName, "r"))==NULL)
        {
                printf ("The file %s could not be opened\n",FileName);
        }

        for(i=0;i<GYMNASTS;i++)
                Result[i]=CalculateScore(DataFile);

        fclose(DataFile);

        for(x=0;x<GYMNASTS;x++)
                StoreData(Result[x]);

Quote:
}

Gymnast CalculateScore(FILE *DataFile)
{
        Gymnast Results;
        int i;

        fgets(Results.Name, 40, DataFile);
        fgets(Results.Country, 30, DataFile);
        for(i=0;i<NUMBEROFJUDGES;i++)
        {
                fscanf(DataFile,"%f",Results.JudgeScores[i]);
        }

        i=0;
        float min;
        float max;
        float sum;

        sum=0; min=1000000; max=0;

        for(i=0;i<NUMBEROFJUDGES;i++)
        {
                sum += Results.JudgeScores[i];

                if(Results.JudgeScores[i] > max)
                        max = Results.JudgeScores[i];

                if(Results.JudgeScores[i] < min)
                        min = Results.JudgeScores[i];
        }
        Results.FinalScore = (sum - (max + min))/(NUMBEROFJUDGES-2);
        return Results;

Quote:
}

void StoreData(Gymnast Result)
{
        char FileName[50];
        FILE *WriteFile;

        puts("Please enter the filename to store data into :");
        gets(FileName);

        WriteFile = fopen(FileName, "a");

        fputs(Result.Name,WriteFile);

        /* then instuctions to print country and final score to file */

Quote:
}



Thu, 22 May 2003 03:00:00 GMT  
 Data Input/Output


Quote:
> Could someone please tell me why the following program doesn't work? I
> have read through it several times and cant see anything obviously wrong
> with it.

> Thanks,
> Jamie Fraser

> ------

> #include <stdio.h>
> #include "c:\My Documents\Ac1101\Projects\include\genio.h"

Non-standard header. I can compile without so what's it doing here?
Quote:

> #define NUMBEROFJUDGES 5
> #define GYMNASTS 15

> typedef struct
> {
> char Name[40], Country[30];
> int Age;
> float JudgeScores[NUMBEROFJUDGES];
> float FinalScore;
> } Gymnast;

> Gymnast CalculateScore(FILE *DataFile);
> void StoreData(Gymnast);

> void main(void)

That's int main(void)

Quote:
> {
> char FileName[50];
> FILE *DataFile;
> int i,x;
> Gymnast Result[GYMNASTS];

> puts ("Please enter the name of the file containing Gymnast
> Data:");
> gets (FileName);

gets is a bug waiting to happen but may suffice for temporary program.

Quote:

> DataFile = fopen(FileName, "r");

> if((DataFile = fopen(FileName, "r"))==NULL)
> {
> printf ("The file %s could not be opened\n",FileName);
> }

Why let the program continue using a NULL pointer? Get outta here now.

Quote:

> for(i=0;i<GYMNASTS;i++)
> Result[i]=CalculateScore(DataFile);

This method seems odd to me but works. I find it better to pass pointers to
structs.

Quote:

> fclose(DataFile);

> for(x=0;x<GYMNASTS;x++)
> StoreData(Result[x]);
> }

> Gymnast CalculateScore(FILE *DataFile)
> {
> Gymnast Results;
> int i;

> fgets(Results.Name, 40, DataFile);

You should check the return value from fgets because the file may have been
empty or truncated.

Quote:
> fgets(Results.Country, 30, DataFile);
> for(i=0;i<NUMBEROFJUDGES;i++)
> {
> fscanf(DataFile,"%f",Results.JudgeScores[i]);

Here is your bug. fscanf will not read the new line at the end of the last
score, consequently when you read the name of the next gymnast you will only
get the new line which corresponds to a zero length string when read with
fgets. You should also check the return value from fscanf because it will
return the number of fields converted and assigned.

A better solution is to use fscanf and strtod which will tell you where the
conversion failed.

Quote:
> }

> i=0;
> float min;
> float max;
> float sum;

How does this compile? Unless you are using a C++ compiler sneakily? These
should be moved to the top of the function.

- Show quoted text -

Quote:

> sum=0; min=1000000; max=0;

> for(i=0;i<NUMBEROFJUDGES;i++)
> {
> sum += Results.JudgeScores[i];

> if(Results.JudgeScores[i] > max)
> max = Results.JudgeScores[i];

> if(Results.JudgeScores[i] < min)
> min = Results.JudgeScores[i];
> }
> Results.FinalScore = (sum - (max + min))/(NUMBEROFJUDGES-2);
> return Results;
> }

> void StoreData(Gymnast Result)
> {
> char FileName[50];
> FILE *WriteFile;

> puts("Please enter the filename to store data into :");
> gets(FileName);

See earlier comment about gets.

Quote:

> WriteFile = fopen(FileName, "a");

Always check the return value from fopen, what if FileName is not a valid
filename?

Quote:

> fputs(Result.Name,WriteFile);

You don't seem to close this file anywhere.

Quote:

> /* then instuctions to print country and final score to file */
> }

--
John Castle
Too err is human, to really louse things up takes a computer!



Thu, 22 May 2003 03:00:00 GMT  
 Data Input/Output

Quote:

> Could someone please tell me why the following program doesn't work? I
> have read through it several times and cant see anything obviously wrong
> with it.

What should it do? What does it do instead?

Quote:
> #include <stdio.h>
> #include "c:\My Documents\Ac1101\Projects\include\genio.h"

This makes the program impossible to compile on pretty much any
machine. Do you need it?

Quote:
> #define NUMBEROFJUDGES 5
> #define GYMNASTS 15

> typedef struct
> {
>    char Name[40], Country[30];
>    int Age;
>    float JudgeScores[NUMBEROFJUDGES];
>    float FinalScore;
> } Gymnast;

> Gymnast CalculateScore(FILE *DataFile);
> void StoreData(Gymnast);

> void main(void)

This is obviously wrong. Change it to int main(void).

Quote:
> {
>    char FileName[50];
>    FILE *DataFile;
>    int i,x;
>    Gymnast Result[GYMNASTS];

>    puts ("Please enter the name of the file containing Gymnast Data:");
>    gets (FileName);

Never use gets() unless you can guarantee that it won't overflow
your buffer.

Quote:
>    DataFile = fopen(FileName, "r");

>    if((DataFile = fopen(FileName, "r"))==NULL)

You are trying to open FileName twice. Did you really mean to do
that?

Quote:
>    {
>            printf ("The file %s could not be opened\n",FileName);

This isn't enough. If the file isn't open, don't execute the code
below. #include <stdlib.h>, then replace this block by

{
    fprintf(stderr, "The file %s could not be opened, exiting.\n",
            FileName);
    exit(EXIT_FAILURE);

Quote:
}
>    }

>    for(i=0;i<GYMNASTS;i++)
>            Result[i]=CalculateScore(DataFile);

>    fclose(DataFile);

>    for(x=0;x<GYMNASTS;x++)
>            StoreData(Result[x]);

return 0;

Quote:
> }

> Gymnast CalculateScore(FILE *DataFile)
> {
>    Gymnast Results;
>    int i;

>    fgets(Results.Name, 40, DataFile);

Replace the magic "30" by sizeof Results.Name. And you should check
fgets()'s return value for NULL in a real program.

Quote:
>    fgets(Results.Country, 30, DataFile);
>    for(i=0;i<NUMBEROFJUDGES;i++)
>    {
>            fscanf(DataFile,"%f",Results.JudgeScores[i]);

And fscanf()'s return value, too.

Quote:
>    }

>    i=0;
>    float min;
>    float max;
>    float sum;

C89 doesn't allow declarations after other statements. Are you sure
you use your compiler in C, not C++, mode?

Quote:
>    sum=0; min=1000000; max=0;

If you #include <limits.h>, you can use FLT_MAX instead of the magic
1000000 here.

Quote:
>    for(i=0;i<NUMBEROFJUDGES;i++)
>    {
>            sum += Results.JudgeScores[i];

>            if(Results.JudgeScores[i] > max)
>                    max = Results.JudgeScores[i];

>            if(Results.JudgeScores[i] < min)
>                    min = Results.JudgeScores[i];
>    }

You could combine these two loops into one, but that's not too
important.

Quote:
>    Results.FinalScore = (sum - (max + min))/(NUMBEROFJUDGES-2);
>    return Results;
> }

> void StoreData(Gymnast Result)
> {
>    char FileName[50];
>    FILE *WriteFile;

>    puts("Please enter the filename to store data into :");
>    gets(FileName);

>    WriteFile = fopen(FileName, "a");

>    fputs(Result.Name,WriteFile);

>    /* then instuctions to print country and final score to file */
> }

Gergo

--
"Ignorance is the soil in which belief in miracles grows."
-- Robert G. Ingersoll



Thu, 22 May 2003 03:00:00 GMT  
 Data Input/Output

Quote:

>> DataFile = fopen(FileName, "r");

>> if((DataFile = fopen(FileName, "r"))==NULL)
>> {
>> printf ("The file %s could not be opened\n",FileName);
>> }

> Why let the program continue using a NULL pointer? Get outta here now.

Good comments, but it's also worth noting that the above is wrong
because it makes two calls to fopen.  The following makes both your
correction and mine:

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

[...]

    DataFile = fopen(FileName, "r");

    if (DataFile == NULL)
    {
        fprintf(stderr, "The file %s could not be opened\n",FileName);
        exit(EXIT_FAILURE);
    }

Quote:
>> for(i=0;i<GYMNASTS;i++)
>> Result[i]=CalculateScore(DataFile);

> This method seems odd to me but works. I find it better to pass pointers to
> structs.

But you don't explain why.  It's less logical to do so on face, but of
course it reduces overhead, sometimes significantly.  In this case, I
don't think it's significant, so I wouldn't've commented personally.

Quote:
>> float min;
>> float max;
>> float sum;

> How does this compile? Unless you are using a C++ compiler sneakily? These
> should be moved to the top of the function.

Many C compilers do support this as an extension, which (I believe) is
why it was codified in C99.  

Quote:
>> void StoreData(Gymnast Result)
>> {
>> char FileName[50];

   char FileName[50] = "";

If fgets fails and leaves FileName unchanged, this code will no longer
use uninitialized data.

Quote:
>> FILE *WriteFile;

>> puts("Please enter the filename to store data into :");
>> gets(FileName);
> See earlier comment about gets.

And to add the relevant code:

   fgets(FileName, sizeof FileName, stdin);

--

http://www.naisbodo.com/



Thu, 22 May 2003 03:00:00 GMT  
 Data Input/Output
See annotations in code:


Quote:

>Could someone please tell me why the following program doesn't work? I
>have read through it several times and cant see anything obviously wrong
>with it.

>Thanks,
>Jamie Fraser

>------

>#include <stdio.h>
>#include "c:\My Documents\Ac1101\Projects\include\genio.h"

>#define NUMBEROFJUDGES 5
>#define GYMNASTS 15

>typedef struct
>{
>    char Name[40], Country[30];
>    int Age;
>    float JudgeScores[NUMBEROFJUDGES];
>    float FinalScore;
>} Gymnast;

>Gymnast CalculateScore(FILE *DataFile);
>void StoreData(Gymnast);

>void main(void)

Others have corrected to int main(void)
Quote:
>{
>    char FileName[50];
>    FILE *DataFile;
>    int i,x;
>    Gymnast Result[GYMNASTS];

>    puts ("Please enter the name of the file containing Gymnast
>Data:");
>    gets (FileName);

>    DataFile = fopen(FileName, "r");

>    if((DataFile = fopen(FileName, "r"))==NULL)

Others have pointed out you cannot open the file twice with fopen().
Quote:
>    {
>            printf ("The file %s could not be opened\n",FileName);
>    }

>    for(i=0;i<GYMNASTS;i++)
>            Result[i]=CalculateScore(DataFile);

While not incorrect, it is extremely unusual and inefficient to
consume the storage and processing time to pass structures back and
forth between functions.  Most would use a pointer to struct instead.

- Show quoted text -

Quote:

>    fclose(DataFile);

>    for(x=0;x<GYMNASTS;x++)
>            StoreData(Result[x]);
>}

>Gymnast CalculateScore(FILE *DataFile)
>{
>    Gymnast Results;
>    int i;

>    fgets(Results.Name, 40, DataFile);
>    fgets(Results.Country, 30, DataFile);
>    for(i=0;i<NUMBEROFJUDGES;i++)
>    {
>            fscanf(DataFile,"%f",Results.JudgeScores[i]);

MAJOR problem here.  fscanf needs the address of the variable to store
the parsed value in.
Quote:
>    }

>    i=0;
>    float min;
>    float max;
>    float sum;

Another serious problem.  In C89, you cannot declare new variables
after executable statements.  Your compiler may allow this extension
or you may have chosen to ignore the diagnostic.  It does prevent
those of us using compilers without this extension from testing your
code.
Quote:

>    sum=0; min=1000000; max=0;

>    for(i=0;i<NUMBEROFJUDGES;i++)
>    {
>            sum += Results.JudgeScores[i];

>            if(Results.JudgeScores[i] > max)
>                    max = Results.JudgeScores[i];

>            if(Results.JudgeScores[i] < min)
>                    min = Results.JudgeScores[i];
>    }

Not incorrect but this could have been combined with the previous loop
instead of looping through the scores again.

- Show quoted text -

Quote:
>    Results.FinalScore = (sum - (max + min))/(NUMBEROFJUDGES-2);
>    return Results;
>}

>void StoreData(Gymnast Result)
>{
>    char FileName[50];
>    FILE *WriteFile;

>    puts("Please enter the filename to store data into :");
>    gets(FileName);

>    WriteFile = fopen(FileName, "a");

>    fputs(Result.Name,WriteFile);

>    /* then instuctions to print country and final score to file */
>}

This function gets called GYMNASTS times.  Is it your intent that the
user specify 15 different file names?

<<Remove the del for email>>



Sat, 24 May 2003 14:27:31 GMT  
 
 [ 5 post ] 

 Relevant Pages 

1. Data Input/Output - New Code- disregard last message

2. input, output, input/output parameters?????

3. 2 questions on data input and output

4. input and output binary data

5. ANSI C Run Time Erroe to output data to the output file

6. Unable to sync input and output streams

7. function with array input/output

8. Hashing long input strings into short output strings

9. File input output

10. Redirecting input and output of a program

11. How to Directly Read Piped-Output as Input in C

12. Converting HTML form input to fax output

 

 
Powered by phpBB® Forum Software