Please don't quote signatures. Trim quoted material to what's
necessary for your followup to make sense to someone who hasn't see
the parent article.
Here is what I have so far, won't even compile. And I am:
#include <stdio.h>
#include <string.h>
Your program doesn't use anything from <string.h>, so this #include
directive is unnecessary. But, as we'll see, it *should*, so yes,
you'll need this #include directive.
int AddEntry(struct entry);
At this point, you haven't declared a type "struct entry". Because
it's an incomplete struct type, there are some interesting details
about how this declaration is handled, but that's not relevant; this
is not what you want, and you'll need to fix it.
struct{
char fName[51];
char lName[51];
char Phone[13];
}Entry;
Now you declare a struct object, but you *still* haven't declared a
type "struct entry". In fact, you don't declare "struct entry" (other
than as an incomplete type) anywhere in your program. What you've
done here is (a) declared an anonymous struct type, and (b) declared a
single object of that type, named "Entry". If all you want is a
single object, that might be sensible thing to do (or you might as
well declare the members as individual variables). But since you want
to pass it as an argument to a function, you should give the type a
name when you declare it.
Change the above declaration to:
struct entry {
char fName[51];
char lName[51];
char Phone[13];
};
and place it *above* the function declaration, so AddEntry knows what
a "struct entry" is. (More precisely, so the compiler knows what a
"struct entry" is when it processes the function declaration.)
(There's another common style that uses a typedef to give a structure
type a one-word name, but I won't get into that here.)
This creates a *named* type, called "struct entry". Note that it only
declares the type; unlike your original declaration, it doesn't
declare a variable of that type. I made that change because the
object (note: I'm using the terms "object" and "variable"
interchangeably) doesn't need to be global (more precisely, at file
scope). You can declare it inside the main() function.
Ok, but "int main(void)" is more explicit, and is preferred.
Add here:
struct entry Entry;
Though you might pick a more descriptive name than "Entry". In a real
program, you're probably going to have more than one object of that
type; each one should have a name that describes that object.
Entry.fName = "Fred";
Entry.lName = "Flintstone";
Entry.Phone = "123-456-7890";
If the members were char* pointers, you could do this, but you can't
assign to an array. Read section 6 of the comp.lang.c FAQ,
<
http://www.c-faq.com/>. You can use the strcpy function here:
strcpy(Entry.fName, "Fred");
retCode = AddEntry(Entry);
You don't do anything with the value of retCode. That's ok in a small
sample program, but in a real program you'll probably want to do
something based on the result.
You should add "return 0;" before the closing brace of the main
function. It's not strictly required, but it's a good idea. You can
do more elaborate things if you want your program to report whether it
succeeded or failed.
}
//int AddEntry(char fName[51], char lName[51], char Phone[13])
It's best to avoid "//" comments when posting to Usenet. The C99
standard supports them, as do most exsting pre-C90 compilers (at least
in some mode), but they're not 100% portable. Also, Usenet software
commonly wraps long lines. Wrapping a "//" comment typically creates
a syntax error; a wrapped "/* ... */" comment typically is still a
valid comment.
int AddEntry(struct entry)
In a function declaration (without a body), you only need the types of
the parameters; the names are optional. In a function definition,
though, you have to provide names for the parameters, so the body of
the function can refer to them.
Let's call it "arg", for "argument":
int AddEntry(struct entry arg)
{
FILE *fp;
fp = fopen("AppData.dat", "a"); /* open file for writing */
You're opening the file for appending, not just writing. Either
mention that in the comment, or just drop the comment; any reader who
doesn't know what fopen() does is going to have bigger problems.
Comments like that are ok if you're trying to convince someone that
you know what you're doing, but they're not useful in providing actual
information to the reader.
You don't check whether the fopen() call succeeded. It returns a null
pointer (NULL) if it fails. Always check the result of fopen(), and
take some corrective action if it fails -- even if the corrective
action is to terminate the program with an error message.
fprintf(fp, "%s", fName); /* write some info */
fprintf(fp, "\t");
fprintf(fp, "%s", lName);
fprintf(fp, "\t");
fprintf(fp, "%s", Phone);
fprintf(fp, "\n");
The references to fName, lName, and Phone assume that they exist as
individual variables. They don't; they're members of a structure.
Now that that structure has been declared properly, you can refer to
them as arg.fName and so forth.
These six printf statements can be reduced to one:
fprintf(fp, "%s\t%s\t%s\n", arg.fName, arg.lName, arg.Phone);
fprintf() can fail. Combining the six printfs into one makes it
easier to check the result, and take some corrective action on
failure.
fclose(fp); /* close the file */
Another vacuous coment.
And fclose() can also fail. Consult the documentation for each of
these functions to find out how they indicate failure; it's different
for different functions.
You always return the same value. If that's all you're going to do,
the function might as well not return a value at all (i.e., you can
declare it to return void rather than int). Or you can use the return
value to indicate to the caller whether the function succeeded or
failed. Decide how you want to do this, and *document* your
convention, for example in a comment associated with the function
declaration. One convention is to return 0 (false) for failure, 1
(true) for success. Another convention, since there are more ways to
fail than to succeed, is to return 0 for success, and some non-zero
value for failure; different values can indicate different kinds of
failure.
The values 51 and 13 are "magic numbers"; it's not at all clear what
they mean, or why you chose those particular values. Declare them as
constants, probably as preprocessor macros:
#define NAME_MAX 51
#define PHONE_MAX 13
Finally, you should learn to understand your compiler's diagnostic
messages (which you didn't show us). The details will vary depending
on which compiler you're using, but any decent compiler will print
messages that will tell an experienced programmer what the problem is,
though they may not always be obvious to a newbie. (They may not be
obvious to an experienced programmer if the compiler writer has
written poor messages, which does happen.)
*Before* you start correcting your code, compile it again, and pay
close attention to what your compiler tells you. If possible, set
options on your compiler to tell it to print lots of verbose warnings.
You'll probably find that your compiler's error and warning messages
will tell you just about everything about your code that I've just
told you, *if* you can understand how it's telling you. Take a look
at what your compiler tells you, and compare it to what I've told you
here.
Note that syntax errors can often confuse the compiler, and make some
of the following messages meaningless. A syntax error is, for
example, something like a missing semicolon or a mismatched
parenthesis. For other errors, like type mismatches, it's generally
easier for the compiler to recover and guess what you meant in the
following code. Fix the syntax errors first, so you can get better
messages for other errors. The very first error message the compiler
gives you is the most reliable; following messages may be the result
of the compiler's confusion.
(For gcc, "-ansi -pedantic -Wall -W -O3" is a good start. ("-O3"
enables optimization; a side effect is that the compiler performs more
analysis and can detect more problems in your code.) For other
compilers, check your documentation.)
