| re: please comment
hellfire wrote:
[color=blue]
> Below is a pretty usless progam but being a begginer
> and working form a book i need some one to comment
> on it IE: is it set out right easy to follow i tried
> to use every thing i know to data your comments will
> be appricated thanks for your time[/color]
Please try to use proper punctuation when posting here. If we can't
understand what you are saying, we can't help. Also, if you aren't
willing to put forth the effort to make your messages coherent, many
people will not be willing to put forth the effort to try to figure out
what you are saying, and will simply skip your message. Keep in mind
that this is a technical discussion forum, not a social forum. It's one
of the more formal corners of the Internet.
[color=blue]
>
> /* VB1.C PROGRAM JUST TO MAKE DESISIONS */
> #include<stdio.h>
>
> /* DECLARE VARIABLES */
> char line[3];
> char sex;
> char choice;[/color]
Global variables are best avoided. These in particular seem to be wholly
unnecessary.
[color=blue]
>
> /* DECLARE FUNCTION PROTOTYPES */
> void get_line(void);
> void get_sex(void);
> void get_choice(void);[/color]
I would expect a function with a name beginning with the word "get" to
return something. For example:
char *get_line(void);
char get_sex(void);
char get_choice(void);
[color=blue]
> int main(void)
>
> {
> printf("Enter m for male f for female: ");[/color]
Output in C is often line-buffered. That means that the I/O system will
collect characters in a buffer until the end of a line, and at that
point will send the characters to their next destination. That means
that your prompt may not be seen right away. To fix this, either add a
newline:
printf("Enter m for male f for female:\n");
or flush the output:
printf("Enter m for male f for female: ");
fflush(stdout);
[color=blue]
> get_sex();[/color]
The organization here is a little strange. I would expect get_sex() to
take on the entire task of determining the sex of the subject. How it
does this is not important to the rest of the program. It just so
happens that it reads it from stdin, but it could just as easily read it
from a file, an environment variable, or choose it randomly. So, main()
should probably not be printing the prompt - that should be a job for
get_sex(). main() doesn't know or care how get_sex() is going to
determine the sex, and shouldn't make any assumptions about it.
Also, you need to get the sex out of get_sex() somehow. You already know
this, and have been using a global variable for it. Trust me when I say
this is a bad idea. Return the value instead.
char sex; /* Add at the beginning of main */
/* ... */
sex = get_sex();
[color=blue]
>
> if(sex == 'm')[/color]
Some people would suggest writing this the other way around:
if ('m' == sex)
in order to avoid a common bug: using '=' instead of '=='.
[color=blue]
> {
> printf("are you a master y for yes n for no; ");
> get_choice();[/color]
Same comments as for sex above (flush the output, probably move the
prompt into get_choice(), return the value).
[color=blue]
>
> if(choice == 'y')
> printf("you are a Master:\n");
> else
> printf("You are a Mr:\n");
> }
>
> else if(sex == 'f')
> {
> printf("are you a miss y for yes n for no: ");
> get_choice();[/color]
And again.
[color=blue]
>
> if(choice == 'y')
> printf("You are a Miss:\n");
> else
> printf("You are a Mrs:\n");
> }
> return 0;
> }
> /* FUNCTION DEFINITIONS */
> void get_line(void)
> {
> fgets(line, sizeof(line),stdin);[/color]
What if the user enters a longer line? What if end-of-file is encountered?
[color=blue]
> }[/color]
When you aren't using global variables anymore, getting a line out of a
function can be tricky. Some options are to pass in a pointer to an
array in which to store the line, to return a pointer to a local static
array, or to return a pointer to a dynamic array:
void get_line(char buf[], size_t len)
{
fgets(buf, len, stdin);
}
char *get_line(void)
{
static char buf[100];
fgets(buf, sizeof(buf), stdin);
return buf;
}
char *get_line(void)
{
char *p = malloc(100);
if (p != NULL)
{
fgets(p, 100, stdin);
}
return p;
}
These all omit error checking and recovery. Each has advantages and
disadvantages. The first is probably preferable in most cases. The
caller is responsible for freeing the dynamic memory in the third case.
-Kevin
--
My email address is valid, but changes periodically.
To contact me please use the address from a recent posting.
| 
