writing strings instead of rabish to file using fprintf

[hpy_awad]

I am writing stings ((*cust).name),((*cust).address)to a file using
fgets but rabish is being wrote to that file ? Look to my source
please and help me finding the reason why this rabish is being
written.


/* Book name :
File name : E:\programs\cpp\iti01\ch10\ex09_5p1.cpp
Program discription: Adding name,Address to customer_record SETUP
PROGRAM

*/
#include <conio.h>
#include <stdio.h>
#include <stdlib.h>
struct customer_record
{
int customer_no;
int no_of_weeks;
char tv_type;
char name[40];
char address[60];

};

main()
{
clrscr();
printf ("-----------------------------------------------------------------------------");
printf ( "\nThis is run of program") ;
printf ( "E:\programs\\cpp\\iti01\\ch09\\ex09_5p1.cpp\n");
printf ( "
-------------------------------------------\n");

void customer_input(struct customer_record *cust);
struct customer_record customer;
char another_customer;

//open file for output
FILE *fp_rental;

if ((fp_rental=fopen("input","w"))==NULL)
{
printf ("\n can not open 'input' for output");
printf ("\n Program terminated");
exit (0);
}

// do while more customers

do {
//input customer date
customer_input(&customer);
//write customer date to file
fprintf(fp_rental,"%4d %2d %c %s %s",customer);
printf("\n Another customer");
scanf("\n");
scanf("%c",&another_customer);
} while ((another_customer)=='y');
//write end of record
fprintf(fp_rental,"%4d %2 d %c %s %s",9999,99,' ',' ',' ');
//clsoe files
fclose(fp_rental);
return 0;
}





void customer_input(struct customer_record *cust)
{
printf("\n Enter customer number : ");
scanf ("%4d",&(*cust).customer_no);
printf("\n Enter number of weeks : ");
scanf ("%2d",&(*cust).no_of_weeks);
printf("\n Enter tv type : ");
scanf ("\n");
scanf ("%c",&(*cust).tv_type);
printf("\n Enter customer name : ");
scanf ("\n");
//scanf ("%c",&(*cust).name);
gets((*cust).name);
printf("\n Enter customer address : ");
scanf ("\n");
//scanf ("%s",&(*cust).address);
gets((*cust).address);
}

 

[Lew Pitcher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I am writing stings ((*cust).name),((*cust).address)to a file using
fgets

I hope not. fgets() doesn't write to files, it reads from them.

but rabish is being wrote to that file ? Look to my source
please and help me finding the reason why this rabish is being
written.


/* Book name :
File name : E:\programs\cpp\iti01\ch10\ex09_5p1.cpp
Program discription: Adding name,Address to customer_record SETUP
PROGRAM

*/
#include <conio.h>

conio.h is not a standard C header. I don't know what it defines (nor do
I care); it could define something that causes a problem with your program.

#include <stdio.h>
#include <stdlib.h>
struct customer_record
{
int customer_no;
int no_of_weeks;
char tv_type;
char name[40];
char address[60];

};

main()

ITYM
int main(void)
which would be the closest to your intent for a C99 compliant compiler.
If you aren't using a C99 compliant compiler, then this program isn't
written in C, or at least is compilable as a C program.

{
clrscr();

clrscr() is not a standard C function, and you haven't provided the
source for it. This function could cause a problem with your program,
and we won't be able to help diagnose it.

printf ("-----------------------------------------------------------------------------");
printf ( "\nThis is run of program") ;
printf ( "E:\programs\\cpp\\iti01\\ch09\\ex09_5p1.cpp\n");

You really need to understand escape sequences. What will the escape
sequence \p (as in "E:\p") print?

printf ( "
-------------------------------------------\n");

void customer_input(struct customer_record *cust);
struct customer_record customer;
char another_customer;

I hope that you are compiling with a C99 compliant compiler, otherwise
this is illegal, and would cause compilation to fail.

//open file for output
FILE *fp_rental;

if ((fp_rental=fopen("input","w"))==NULL)
{
printf ("\n can not open 'input' for output");
printf ("\n Program terminated");
exit (0);
}

// do while more customers

do {
//input customer date
customer_input(&customer);
//write customer date to file
fprintf(fp_rental,"%4d %2d %c %s %s",customer);

Here's the line that's causing your problem.

You ask fprintf() to print
a) a four digit number,
b) a two digit number,
c) a single character,
d) a string, and
e) another string
but you don't give fprintf() any of those things. Instead, you give
fprintf() a "customer". You've just invoked "undefined behaviour", where
the program can do /anything/, including generating inappropriate results.

You really mean to write
fprintf(fp_rental,"%4d %2d %c %s %s",
customer.customer_no,
customer.no_of_weeks,
customer.tv_type,
customer.name,
customer.address);

printf("\n Another customer");
scanf("\n");
scanf("%c",&another_customer);
} while ((another_customer)=='y');
//write end of record
fprintf(fp_rental,"%4d %2 d %c %s %s",9999,99,' ',' ',' ');

Close, but no cigar.
1) you have an improper format string: %2 isn't proper
2) the third and fourth data arguments are not strings, they are single
characters, and
3) you have one too many data arguments for the format string given

//clsoe files
fclose(fp_rental);
return 0;
}





void customer_input(struct customer_record *cust)
{
printf("\n Enter customer number : ");
scanf ("%4d",&(*cust).customer_no);

1) what if I enter "abcdefgh" to the customer number prompt?
2) &(*cust).customer_no ?? ITYM &(cust->customer_no)

printf("\n Enter number of weeks : ");
scanf ("%2d",&(*cust).no_of_weeks);

same as above

printf("\n Enter tv type : ");
scanf ("\n");
scanf ("%c",&(*cust).tv_type);

same as above

printf("\n Enter customer name : ");
scanf ("\n");
//scanf ("%c",&(*cust).name);
gets((*cust).name);

same as above, with an additional caveat:
It isn't a good idea to mix input methods. It can work, but you /really/
have to understand what you are doing in order to make it work
correctly, and you haven't demonstrated that you understand what you are
doing yet.

printf("\n Enter customer address : ");
scanf ("\n");
//scanf ("%s",&(*cust).address);
gets((*cust).address);

same as above

}

- --

Lew Pitcher, IT Consultant, Enterprise Application Architecture
Enterprise Technology Solutions, TD Bank Financial Group

(Opinions expressed here are my own, not my employer's)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (MingW32)

iD8DBQFArhyaagVFX4UWr64RAgM0AKDbSs73zJJ4Kg7pAFXNstaFIqiPjQCaAk75
NJGZFZrQw+fpsh+dQlkDXo0=
=pyVk
-----END PGP SIGNATURE-----

 

[Thomas Matthews]

I am writing stings ((*cust).name),((*cust).address)to a file using
fgets but rabish is being wrote to that file ? Look to my source
please and help me finding the reason why this rabish is being
written.

Lew Pitcher has already posted an excellent reply, so I will
indicate some alternative methods for what you are doing.

/* Book name :
File name : E:\programs\cpp\iti01\ch10\ex09_5p1.cpp

Paths should not be inside files. Files don't know where they
are located. Their location may change without their knowledge.
If this file moves, then this comment will have to change.

Program discription: Adding name,Address to customer_record SETUP
PROGRAM

*/
#include <conio.h>

This is a platform specific header file.
I suggest you do without it.

#include <stdio.h>
#include <stdlib.h>
struct customer_record
{
int customer_no;
int no_of_weeks;
char tv_type;
char name[40];
char address[60];

};

"Magic Numbers". That's what 40 and 60 are called above.
A common guideline is to use named constants:
#define MAX_NAME_LENGTH 40
#define MAX_ADDRESS_LENGTH 60

....
char name[MAX_NAME_LENGTH];
char address[MAX_ADDRESS_LENGTH];

Also, is tv_type a letter or just a small number or perhaps
an enumeration? If it is a number, use either int or unsigned
int. Unless you are writing code for a system that has a small
amount of memory, don't use short or char for numbers. For
many platforms, the processor is more efficient using int than
char or short.

If your no_of_weeks field cannot be negative, I suggest you
use an unsigned int. I haven't run across a occasion where
someone checked out a video for -2 weeks.

And similarly with customer number. However, some systems use
a negative customer number as an indicator. (Such as customer
doesn't exist anymore.)

main()
{
clrscr();

No need to clear the screen. Also, on windowing platforms,
are you clearing the whole screen or the window?

printf ("-----------------------------------------------------------------------------");
printf ( "\nThis is run of program") ;
printf ( "E:\programs\\cpp\\iti01\\ch09\\ex09_5p1.cpp\n");
printf ( "
-------------------------------------------\n");

Hey, try this one out:
static const char program_header_text[] =
"--------------------------------------------------\n"
"This is run of program ex09_5p1.cpp\n"
"--------------------------------------------------\n";
fwrite(program_header_txt,
sizeof(char), /* to be explicit */
sizeof(program_header_text) - 1, /* don't write the '\0' */
stdout);

or this:
puts(program_header_text);

void customer_input(struct customer_record *cust);
struct customer_record customer;
char another_customer;

//open file for output
FILE *fp_rental;

if ((fp_rental=fopen("input","w"))==NULL)

To make your program easier to read, split this into more
lines:
const char OUTPUT_FILENAME[] = "input";

fp_rental = fopen(OUTPUT_FILENAME, "w");
if (!fp_rental)
{
fprintf(stderr, "\nCannot open %s for output\n",
OUTPUT_FILENAME);
fprintf(stderr, "Program terminated.\n");
return EXIT_FAILURE;
}
Using a constant literal for the filename allows you to
only have to change one line when that filename is modified.

The main() is a function, so it must return a value.
Two defined values are EXIT_SUCCESS and EXIT_FAILURE, which

{
printf ("\n can not open 'input' for output");
printf ("\n Program terminated");
exit (0);
}

// do while more customers

do {
//input customer date
customer_input(&customer);
//write customer date to file
fprintf(fp_rental,"%4d %2d %c %s %s",customer);

This should only be performed if the customer data is valid.

printf("\n Another customer");
scanf("\n");
scanf("%c",&another_customer);
} while ((another_customer)=='y');

while ( toupper(another_customer) == 'Y');
or
while (tolower(another_customer) == 'y');

//write end of record
fprintf(fp_rental,"%4d %2 d %c %s %s",9999,99,' ',' ',' ');

Magic Numbers and literals. Use named constants.

//clsoe files
fclose(fp_rental);
return 0;

return EXIT_SUCCESS;

}





void customer_input(struct customer_record *cust)
{
printf("\n Enter customer number : ");
scanf ("%4d",&(*cust).customer_no);
printf("\n Enter number of weeks : ");
scanf ("%2d",&(*cust).no_of_weeks);
printf("\n Enter tv type : ");
scanf ("\n");
scanf ("%c",&(*cust).tv_type);
printf("\n Enter customer name : ");
scanf ("\n");
//scanf ("%c",&(*cust).name);
gets((*cust).name);
printf("\n Enter customer address : ");
scanf ("\n");
//scanf ("%s",&(*cust).address);
gets((*cust).address);
}

The scanf function is evil, see the C language
FAQ below.

I suggest that if you have a special input function
for the customer_record, you have a complementary
output function also (and use it). Many programmers
and designers have one set of I/O functions for
file I/O and another for User Interface I/O.

As a matter of encapsulation, I suggest that you
place all of the customer functions into one
module. Create a header file that defines the
customer_record and declares the functions.

--
Thomas Matthews

C++ newsgroup welcome message:
http://www.slack.net/~shiva/welcome.txt
C++ Faq: http://www.parashift.com/c++-faq-lite
C Faq: http://www.eskimo.com/~scs/c-faq/top.html
alt.comp.lang.learn.c-c++ faq:
http://www.raos.demon.uk/acllc-c++/faq.html
Other sites:
http://www.josuttis.com -- C++ STL Library book

 

[Emmanuel Delahaye]

/* Book name :
File name : E:\programs\cpp\iti01\ch10\ex09_5p1.cpp

Why .cpp ? Are you writing in C of not?

Program discription: Adding name,Address to customer_record SETUP
PROGRAM

You are not 'adding', but you create a new file each time. using fopen()
with "a" instead of "w" will add.

*/
#include <conio.h>

Non standard and useless.

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

struct customer_record
{
int customer_no;
int no_of_weeks;
char tv_type;
char name[40];
char address[60];

};

main()

Prefer

int main ()
or
int main (void)

{
clrscr();

Get rid of this.

printf
("-----------------------------------------------------------------------
------"); printf ( "\nThis is run of program") ;
printf ( "E:\programs\\cpp\\iti01\\ch09\\ex09_5p1.cpp\n");
printf ( "
-------------------------------------------\n");

The __FILE__ macro gives the name of the current file.

void customer_input(struct customer_record *cust);

Don't put prototypes in a function. They belong to a header. In your case,
you just have to follow the "define before use" design rule and to change
your layout accordingly. Local functions should be defined with 'static'.

struct customer_record customer;
char another_customer;

//open file for output

Keep in mind that the // comment style has not to be supported by C90
compilers

FILE *fp_rental;

if ((fp_rental=fopen("input","w"))==NULL)

"a" is probably better for your case...

{
printf ("\n can not open 'input' for output");
printf ("\n Program terminated");

The '\n' character is the end of line indicator. Why putting it at the
beginning of the line?

"Sounds illogical, capt'ain". -- Spock.

exit (0);

The standard error value is EXIT_FAILURE...

}

// do while more customers

do {
//input customer date
customer_input(&customer);
//write customer date to file
fprintf(fp_rental,"%4d %2d %c %s %s",customer);

The C language is not that clever! Each "%..." field must have a, explicit
value.

To be able to retrieve the data easily, I suggest a CSV (Comma Separated
Values) format terminated by a '\n' (one by record). In that case, you don't
need the width formatters :

"%d,%d,%c,%s,%s"

printf("\n Another customer");
scanf("\n");

Ugly! scanf() is not designed to get data from a human ('f' means
'formatted'. Fortunately, humans are not (yet) formatted.)

scanf("%c",&another_customer);

You need to write your own input functions based on fgetc() or fgets().

} while ((another_customer)=='y');
//write end of record
fprintf(fp_rental,"%4d %2 d %c %s %s",9999,99,' ',' ',' ');

The record format should be 'factorised' (a macro is just fine)

You don't expect more than 9999 customers for the progam life ? Software can
survive on different hardware for decades.

//clsoe files
fclose(fp_rental);
return 0;
}

void customer_input(struct customer_record *cust)
{
printf("\n Enter customer number : ");

You need a fflush (stdout) to be sure that the caracters are outputted on
time.

scanf ("%4d",&(*cust).customer_no);

Once again, you need a more solid function of your own, based on fgets() and
strtoul(), for example.

printf("\n Enter number of weeks : ");
scanf ("%2d",&(*cust).no_of_weeks);
printf("\n Enter tv type : ");
scanf ("\n");
scanf ("%c",&(*cust).tv_type);
printf("\n Enter customer name : ");
scanf ("\n");
//scanf ("%c",&(*cust).name);
gets((*cust).name);

No. it's a bug. You really have to learn more about fgets().

printf("\n Enter customer address : ");
scanf ("\n");
//scanf ("%s",&(*cust).address);
gets((*cust).address);
}

Try this. Missing code is here:

http://mapage.noos.fr/emdel/clib.htm
Module IO

/* Book name :
File name :
Program discription: Adding name,Address to customer_record SETUP
PROGRAM

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

#include "ed/inc/io.h"

struct customer_record
{
int customer_no;
int no_of_weeks;
char tv_type;
char name[40];
char address[60];

};

static void customer_input (struct customer_record *cust)
{
unsigned long n;

printf (" Enter customer number: ");
fflush (stdout);
get_ul (&n);
cust->customer_no = (int) n;

printf (" Enter number of weeks: ");
fflush (stdout);
get_ul (&n);
cust->no_of_weeks = (int) n;

printf (" Enter tv type: ");
fflush (stdout);
cust->tv_type = get_c ();

printf (" Enter customer name: ");
fflush (stdout);
get_s (cust->name, sizeof cust->name);

printf ("Enter customer address: ");
fflush (stdout);
get_s (cust->address, sizeof cust->address);
}

int main (void)
{
int ret = EXIT_SUCCESS;

printf ("-------------------------------------------\n"
"This is run of program '%s'\n"
"-------------------------------------------\n"
,__FILE__);
{

/* open file for output */
FILE *fp_rental = fopen ("input", "w");

if (fp_rental != NULL)
{
#define RECORD_FORMAT "%4d,%2d,%c,%s,%s\n"
struct customer_record customer;
char another_customer;

/* do while more customers */
do
{
/* input customer date */
customer_input (&customer);
/* write customer date to file */
fprintf (fp_rental
,RECORD_FORMAT
,customer.customer_no
,customer.no_of_weeks
,customer.tv_type
,customer.name
,customer.address
);

printf ("Another customer (y/n): ");
fflush (stdout);

another_customer = get_c ();
}
while ((another_customer) == 'y');

/* write end of record */
fprintf (fp_rental
,RECORD_FORMAT
,9999, 99, ' ', " ", " ");

/* close files */
fclose (fp_rental);
#undef RECORD_FORMAT
}
else
{
printf ("can not open 'input' for output\n");
printf ("Program terminated\n");
ret = EXIT_FAILURE;
}
}
return ret;
}

 

[red floyd]

I am writing stings ((*cust).name),((*cust).address)to a file using
fgets but rabish is being wrote to that file ? Look to my source
please and help me finding the reason why this rabish is being
written.
[redacted]

1. Opening a file named "input" for output is counterintuitive and
not good for maintainability.
2. Learn about the -> operator, for readability purposes, if nothing
else.

 

[Emmanuel Delahaye]

In 'comp.lang.c', Thomas Matthews

struct customer_record
{
int customer_no;
int no_of_weeks;
char tv_type;
char name[40];
char address[60];

};

"Magic Numbers". That's what 40 and 60 are called above.
A common guideline is to use named constants:
#define MAX_NAME_LENGTH 40
#define MAX_ADDRESS_LENGTH 60

...
char name[MAX_NAME_LENGTH];
char address[MAX_ADDRESS_LENGTH];

For this example, I disagree. The important thing is that the so-called magic
number (that have to exist in somme manner) is defined once and only once.
BTW, isn't the array itself the best place to define its size ?

struct customer_record
{
...
char name[40];
char address[60];
};

Is just fine to me, self commenting and easy to maintain. If you want the
size somewhere else, you know how to use sizeof properly.

 

[Gregory Pietsch]

I am writing stings ((*cust).name),((*cust).address)to a file using
fgets but rabish is being wrote to that file ? Look to my source
please and help me finding the reason why this rabish is being
written.


/* Book name :
File name : E:\programs\cpp\iti01\ch10\ex09_5p1.cpp
Program discription: Adding name,Address to customer_record SETUP
PROGRAM

*/
#include <conio.h>

Not standard and not needed here.

#include <stdio.h>
#include <stdlib.h>
struct customer_record
{
int customer_no;
int no_of_weeks;
char tv_type;
char name[40];
char address[60];

};

main()

int main(void)

{
clrscr();

clrscr() is non-standard.

printf ("-----------------------------------------------------------------------------");
printf ( "\nThis is run of program") ;
printf ( "E:\programs\\cpp\\iti01\\ch09\\ex09_5p1.cpp\n");
printf ( "
-------------------------------------------\n");

void customer_input(struct customer_record *cust);
struct customer_record customer;
char another_customer;

//open file for output
FILE *fp_rental;

if ((fp_rental=fopen("input","w"))==NULL)
{
printf ("\n can not open 'input' for output");
printf ("\n Program terminated");
exit (0);
}

// do while more customers

Not a nit, just an observation: Use /* */ comments in C.

do {
//input customer date
customer_input(&customer);
//write customer date to file
fprintf(fp_rental,"%4d %2d %c %s %s",customer);
printf("\n Another customer");
scanf("\n");
scanf("%c",&another_customer);

The scanf() function is tricky to use correctly. I recommend for
amateurs to avoid it and use fgets() plus any of (sscanf, strtod,
strtoul) or similar functions. You never know what kind of input
you're going to get.

} while ((another_customer)=='y');
//write end of record
fprintf(fp_rental,"%4d %2 d %c %s %s",9999,99,' ',' ',' ');

^-- bug here. ^---^-- must be
strings, not

characters, to match
the format
specifier
in
fprintf().

//clsoe files
fclose(fp_rental);
return 0;
}





void customer_input(struct customer_record *cust)
{
printf("\n Enter customer number : ");
scanf ("%4d",&(*cust).customer_no);
printf("\n Enter number of weeks : ");
scanf ("%2d",&(*cust).no_of_weeks);
printf("\n Enter tv type : ");
scanf ("\n");
scanf ("%c",&(*cust).tv_type);
printf("\n Enter customer name : ");
scanf ("\n");
//scanf ("%c",&(*cust).name);
gets((*cust).name);
printf("\n Enter customer address : ");
scanf ("\n");
//scanf ("%s",&(*cust).address);
gets((*cust).address);

Do not use gets(), for it is a tool of the Devil. Use fgets() instead.

I also recommend fflush(stdout); after all the printf()s.

}

I'm sure the listmates will have more.

Gregory Pietsch