The following code is executing alright, although the output file it
generates doesn't look right. The input files is a text file along the
lines of

Ronaldo
Brazil
24
78.0
77.0
82.0
82.0
84.0
Rivaldo
Brazil

etc.etc. (For a total of 15 people).

The output is messed up, and I cant figure out why.
Please can someone help me with this?

-----------

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

#define NUMBEROFJUDGES 5
#define GYMNASTS 15

typedef struct
{
        char Name[50], Country[50];
        int Age;
        float JudgesScores[NUMBEROFJUDGES];
        float FinalScore;

void ReadData(FILE *InputFile, Gymnast Competitors[], int Judges, int
Gymnasts);
void StoreResults(Gymnast Competitors[], FILE *OutputFile);

void main(void)
{
        char FileName[50],FileName2[50];
        FILE *DataFile;
        FILE *OutputFile;
        Gymnast Competitors[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("Cannot open file \n");
        }

        ReadData(DataFile,Competitors,NUMBEROFJUDGES,GYMNASTS);
        fclose(DataFile);

        puts ("Please enter the output filename:");
        gets (FileName2);

        OutputFile=fopen(FileName2,"a");

        StoreResults(Competitors,OutputFile);

void ReadData(FILE *DataFile, Gymnast Competitors[], int Judges, int
Gymnasts)
{
        int i,j;

        for(i=0;i<GYMNASTS;i++)
        {
                fgets(Competitors[i].Name,50,DataFile);
                fgets(Competitors[i].Country,50,DataFile);
                fscanf(DataFile,"%d\n",&Competitors[i].Age);

                for(j=0;j<NUMBEROFJUDGES;j++)
                {
                        fscanf(DataFile,"%f\n",&Competitors[j].JudgesScores[
i]);
                }
        }

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

        for(j=0;j<GYMNASTS;j++)
        {
                sum=0; min=1000000; max=0;

                for(i=0;i<NUMBEROFJUDGES;i++)
                {
                        sum += Competitors[j].JudgesScores[i];

                        if(Competitors[j].JudgesScores[i] > max)
                                max = Competitors[j].JudgesScores[i];

                        if(Competitors[j].JudgesScores[i] < min)
                                min = Competitors[j].JudgesScores[i];
                }
                Competitors[j].FinalScore = (sum - (max +
min))/(NUMBEROFJUDGES-2);
                sum=0;
        }

void StoreResults(Gymnast Competitors[], FILE *OutputFile)
{
        int i;
        for(i=0;i<GYMNASTS;i++)
        {
                fputs(Competitors[i].Name,OutputFile);
                fputs(Competitors[i].Country,OutputFile);
                fprintf(OutputFile,"Final Score: %0.2f\n",
                        Competitors[i].FinalScore);
        }

        fclose(OutputFile);

Please go back and read the comments already made in your other thread.
Do not disregard them.  Most of them still apply.

--

http://www.naisbodo.com/

I strongly suggest you read the comments for the last message since a lot of
both mine and Gergo comments still apply.

[snip]

 void ReadData(FILE *DataFile, Gymnast Competitors[], int Judges, int

I suggest you look carefully at this and see if you can spot why when I run
it the return to the environment fails!

These should be at the top of the function.

Your problem from above will have a significant impact here.

[snip]

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

Im still a beginner (been learning C for about 6 weeks), so Im not really
sure what you mean by this...

Help! Not sure what you mean,

Thanks,

Jamie Fraser

Sorry I should have been more careful in my wording, I was trying to get you
to find the error because sometimes pointing out almost obvious mistakes
doesn't really help.

You're accessing Competitors[NUMBEROFJUDGES-1].JudgesScores[GYMNASTS-1] and
if you look carefully at this you'll see the mistake. When I run this my
machine accesses an area of memory that it owns, but which shouldn't be
modified, the return from main() gets corrupted and an interesting error
occurs in a different place to where the offending line in the code is.

Unfortunately a bug occurring in one place is often caused by something
happening elsewhere. (There are also the strange errors that recompiling
gets rid of.  Then there are the even stranger bugs where modifying the code
and then changing it back to what it was originally makes the original code
work, I've never quite understood this but I've encountered it several times
and attributed it to dodgy Microsoft compilers :-) )

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

Groovy hepcat Jamie Fraser was jivin' on Sun, 3 Dec 2000 19:42:02
-0000 in comp.lang.c.
Data Input/Output - New Code- disregard last message's a cool scene!
Dig it!

  You still haven't fixed this. Remember, backslashes in strings and
character constants are escapes. A backslash modifies the meaning of
the next character. This line, therefore, is severely broken. It will
not compile, even if there is a header called
c:\My Documents\Ac1101\Projects\include\genio.h available. Solution 1:
very simple. Escape the backslashes:

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

Solution 2: also very simple. Change the backslashes to forward
slashes:

#include "c:/My Documents/Ac1101/Projects/include/genio.h"

  Besides, someone pointed out that you don't need this at all, so why
have you chosen to retain it? Why have you chosen to ignore the advice
people have already given you?
  However, you will need these ones:

#include <stdlib.h>
#include <string.h>

(See below.)

  You still haven't fixed this! FCOL, listen to what you are told. Do
as we say, or don't ask for our help again. Why should we help you if
you don't pay attention?
  We keep saying this until we're blue in the face. If you had read at
least a month or two of posts in this newsgroup before posting, as is
customary, you would certainly have seen us tell countless clueless
newbies just like yourself the very same thing. But do any of you ever
pay attention? Do you read the FAQ before posting, like you should? Do
you lurk here for a while before posting, like you should? Do you read
your C manuals to see if they answer your question before posting,
like you should? No. None of you do any of that. And then you wonder
why people seem to be mean to you!

int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)
int main(void)

Get the picture?

  You'll need the following variable:

  char *p;

(See below.)

  You just don't listen, do you? You were also told not to use gets().
I'll tell you again. DON'T USE gets()!!!!! It is an abomination. It
should never have been included in the standard C library ITFP.
Instead, use fgets().
  After you make that change, you'll have to make another. You need to
search for the newline character in the string and eradicate it, since
fgets() retains it, whereas gets() reads it in and discards it. To do
this, you need to use strchr() to return a pointer to it. strchr() is
declared in string.h, so you need to include that. You also need
another variable, a char *, used to store the location of the '\n'.

  p = strchr(FileName, '\n');
  if(p)
    *p = '\0';

  You're opening the file twice here. Why on Earth are you doing that?
Open it once. Test for failure once.

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

  Quit, FCOL! If you don't quit here, you'll be trying to access the
file even though it couldn't be opened. You'll be dereferencing a null
pointer. That is, obviously, extremely bad!

    return EXIT_FAILURE;

You need to include stdlib.h for the definition of EXIT_FAILURE.

  Why waste space on another array? Just re-use the first one,
FileName, and discard this other one.
  You just don't listen, do you? You were also told not to use gets().
I'll tell you again. DON'T USE gets()!!!!! It is an abomination. It
should never have been included in the standard C library ITFP.
Instead, use fgets().
  After you make that change, you'll have to make another. You need to
search for the newline character in the string and eradicate it, since
fgets() retains it, whereas gets() reads it in and discards it. To do
this, you need to use strchr() to return a pointer to it. strchr() is
declared in string.h, so you need to include that. You also need
another variable, a char *, used to store the location of the '\n'.

  p = strchr(FileName, '\n');
  if(p)
    *p = '\0';

  Check the return value of fopen(). *****ALWAYS***** do this. Never
neglect this step. fopen() can fail, and when it does, you must not
try to access the file. The pointer returned will be NULL on failure.
Test for this, and quit or something. At least skip the file accessing
code.

  main() returns an int. So you should return an int. Portable return
values for main() are 0, EXIT_SUCCESS and EXIT_FAILURE (the latter two
defined in stdlib.h).

  return 0;

  This just won't compile at all. You can't put declarations after
executable statements in the innermost block in which they appear. So,
move these declarations to the top of the function.

  And what is this useless statement in aid of? Since you immediately
overwrite these variables (with the same values yet), this statement
is completely superfluous. And what's the point of combining the two
assignments into one statement with the comma operator?

  Don't put multiple statements on one line.

  Don't close the file here. It's bad form. You should close it at the
same function call level at which you opened it; that means in main().

  There may be many other things wrong that I have not spotted. I
didn't run the program all the way through, so I have no idea whether
or not it works with the fixes I suggested. But at least it compiles.
--

Dig the even newer still, yet more improved, sig!

http://alphalink.com.au/~phaywood/
"Ain't I'm a dog?" - Ronny Self, Ain't I'm a Dog, written by G. Sherry & W. Walker.
I know it's not "technically correct" English; but since when was rock & roll "technically correct"?

Ah, but (according to a parched and dusty old copy of N869 (and yes,
I will purchase the real thing (eventually))) preprocessing directives
are executed in translation phase 4, while escape sequences in string
literals are converted in translation phase 5.

Of course, the interpretation of the include string is highly
implementation dependant. Gcc converts any number of consecutive
backslashes into a single forward slash.

I'd certainly go for forward slashes, if possible.

[A multitude of good advice snipped]

How does one determine what is "technically correct"? Languages evolve
on their own and what is "technically correct" differ over geographical,
cultural, social regions, and over time. It certainly isn't mandated by
dictionaries.

  --n