Invalid code test

Hey there.

I've been asked to pick up some errors and mistakes in the following code below.

My problem is that I am C# only and therefore, some of the code in here just doesn't make enough sense (I won't be learning C++ for another couple of weeks).

If anyone can help find as many errors and non-standard problems (and their solutions) as possible, I'd be very much grateful.

I've already spotted a couple.

For example:
#include <stdafx.h> is not included.
void main (void) is not standards compliant and shouldn't be used (although the alternative eludes me).
A missing sem-colon after the typedef.

// ---------------------- //
// Standard Include Files //
// ---------------------- //
#include <stdlib.h>
#include <stdio.h>

// --------------------- //
// Structure Definitions //
// --------------------- //

// Definition of a person
typedef struct PERSON {
     int id;
     char person_name[ 50 ];
     double ability;
     double potential;
}

// Function:   main()
// Purpose:    Main function for the program
void main( void )
{
     FILE *p_file;          // File being read in from
     PERSON *person;          // Person stored in file

     // Open the file to read from
     p_file = fopen( "file.txt", "r" );

     // Read out the person information
     fread( &person, sizeof( person ), 1, p_file );

     // Print the person name to the screen
     printf( "%s\n", person.person_name );

} // main()

Thanks in advance for any help.
DanBAtkinsonAsked:
Who is Participating?

[Webinar] Streamline your web hosting managementRegister Today

x
 
grg99Connect With a Mentor Commented:
Several problems:

#include <stdafx.h>   ....  That's a

void main( void )....  is not quite right, probably should be: int main( int argc, char * argv[] )

typedef struct PERSON {
     int id;
     char person_name[ 50 ];
     double ability;
     double potential;
}

should be:

typedef struct {     int id;    char person_name[ 50 ];   double ability, potential; }    PERSON;


 p_file = fopen( "file.txt", "r" );  .. should check p_file to make sure it is not NULL!!


fread( &person, sizeof( person ), 1, p_file );   ... should check return value

 
     printf( "%s\n", person.person_name );  .. should check return value (but nobody does)

} // main()   ...  should return(0), and close(p_file) first.

ALSO ther's a big problem, you if you use a *PERSON, you have to do a "person = malloc(sizeof(PERSON));"

alternatively declare it as a static structure:    PEERSON person.


 
Thanks in advance for any help.
0
 
ikeworkConnect With a Mentor Commented:
hi DanBAtkinson ,

one problem is you use a pointer to PERSON, but not struct is not instanciated anywhere, use the structure itsself, not a pointer to it,


// Function:   main()
// Purpose:    Main function for the program
void main( void )
{
     FILE *p_file;          // File being read in from
     PERSON person;          // Person stored in file  << here we use the structure, not a pointer to it

     // Open the file to read from
     p_file = fopen( "file.txt", "r" );

     // Read out the person information
     fread( &person, sizeof( PERSON ), 1, p_file );  // << sizeof( PERSON ) gives us the size of the structure, you got the size of the pointer

     // Print the person name to the screen
     printf( "%s\n", person.person_name );

} // main()


good luck :)
ike
0
 
AlexFMCommented:
PERSON *person;

replace with:

PERSON person;

0
Never miss a deadline with monday.com

The revolutionary project management tool is here!   Plan visually with a single glance and make sure your projects get done.

 
ikeworkConnect With a Mentor Commented:
>> fread( &person, sizeof( PERSON ), 1, p_file );  // << sizeof( PERSON ) gives us the size of the structure, you got the size of the pointer

this only works, if the file is binary. if you have a text-file, where values are stored as text (especially the int & double's), then it doesnt work ...

can you show us the contents of your input-file "file.txt"
0
 
DanBAtkinsonAuthor Commented:
Thanks to all for the speedy answering!

Ikework, the contents of file.txt are not binary. Also, its contents are not known.

Can I also ask about fopen?
When looking through the code in VS, it tells me that fopen is now deprecated in favour of fopen_s.

Would this also be a point to make? If so, what would the syntax be for using it? fopen_s("file.txt","r"); ??
0
 
grg99Connect With a Mentor Commented:
The contents of the file have to be binary, as you're reading into a structure with binary components, such as the "double" variables.

If it's a text file, you'd have to use some read method that can convert from text to binary, such as fscanf().

0
 
DanBAtkinsonAuthor Commented:
Thankyou.

How would fscan be implemented in this case?

I have rewritten the code following the comments made here to be the following:

//---------------------------//
// Standard Include Files //
//---------------------------//
#include <stdafx.h>

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

//-------------------------//
// Structure Definitions //
//-------------------------//

// Definition of a player
typedef struct
{
      int id;
      char player_name[ 50 ];
      double ability;
      double potential;
} PLAYER;

// Function: main()
// Purpose: Main function for the program
int main(int argc, char * argv[])
{
      FILE *p_file;            // File being read in from
      PLAYER player;            // Player stored in file

      // Check if the file exists
      if (fopen("file.txt", "r") == NULL)
      {
            //If the file does not exist, display an error and exit
            printf("The file does not exist\n");
            return(0);
      }
      else
      {
            // Open the file to read from
              p_file = fopen("file.txt", "r");
      }

      //Checks the file to make sure that it is of the expected structure
      if (fread(&player,sizeof(PLAYER),1,p_file) == NULL)
      {
            // If the data does not match, display an error and exit
            printf("The read data does not match the structure expected.\n");
            return(0);
      }
      else
      {
            fread(&player,sizeof(PLAYER),1,p_file);
            // Read out the player information
      }

      // Close the file after reading
      fclose(p_file);

      // Print the players name to the screen
      printf("%s\n", player.player_name);

      return(0);
}

The only problem (a warning actually) I have, is that I have a deprecation warning for fopen. If it (fopen_s) is not part of the standard C++ library, as I'm reading, then it won't matter.

If you would be able to give further feedback on the code, then I would be able to award points out. At this point, I believe a points split between grg99 and ikework would be best.
0
 
grg99Connect With a Mentor Commented:
okay, a few little things, but you're getting close.

the fread(...)==NULL should be fread()== sizeof(PLAYER)

but actually if the file is text, you need to use fscanf instead:

items = fscanf( p_file, "%d %s %f %f", &player.id, &player.player_name, &player.ability, &player.potential );
if( items == 4 ) { // okay } else { //didnt read 4 items!! }



0
 
DanBAtkinsonAuthor Commented:
Thankyou.

In the example you use, you mix methods. You use both fread and fscanf. Which one should be used? I'm confused.

Here is the altered code:

         //Checks the file to make sure that it is of the expected structure
         if (fread(&person,sizeof(PERSON),1,p_file) == sizeof(PERSON))
    {
        // If the data does not match, display an error and exit
        printf("The read data does not match the structure expected.\n");
        return(0);
    }
    else
    {
        // Read out the person information
        fread(&person,sizeof(PERSON),1,p_file);

        //Checks the number of values returned by the
        items = fscanf( p_file, "%d %s %f %f", &person.id, &person.person_name, &person.ability, &person.potential );
        if (items==4)
        {
            printf("The was an unexpected number of data values from the file");
        }
    }

Is this right??
0
 
DanBAtkinsonAuthor Commented:
Ooops. Taking fscanf into consideration...

         if (fscanf(&person,sizeof(PERSON),1,p_file) == sizeof(PERSON))
    {
        // If the data does not match, display an error and exit
        printf("The read data does not match the structure expected.\n");
        return(0);
    }
    else
    {
        // Read out the player information
        fscanf(&person,sizeof(PERSON),1,p_file);

        //Checks the number of values returned by the
        items = fscanf( p_file, "%d %s %f %f", &person.id, &person.person_name, &person.ability, &person.potential );

        if (items==4)
        {
            printf("The was an unexpected number of data values from the file");
        }
    }

From this, I get the following error:
error C2664: 'fscanf' : cannot convert parameter 1 from 'PERSON *' to 'FILE *'

Having not used fscanf, and not finding a great deal of help on MSDN, I'm not sure how to go forward.
0
 
DanBAtkinsonAuthor Commented:
I think that using fscanf introduces ambiguities here. By using fread in the first place I think it's specified that binary data is being required.

I think it'll be better to include an assumption that binary is stored in the textfile and that fscanf can be used if that isn't the case.

I'll wrap the question up now with points split between grg99 (300 - 60%) and ike (200 - 40%).

Thanks for the comments everyone. They've been extremely helpful.
0
All Courses

From novice to tech pro — start learning today.