Unseen error

H

hpy_awad

I wrote that example from a book and there is en error in the display
module that it does not showing all the records are entered in the
input module.
I traced with some printf statments without getting the solution . I
think the error in the display module loop condition.
Thanks



#include <stdio.h>
//-exercise 7.5.1 - Goal Statistics
struct football
{
char name[30];
char team[30];
int goals;
};
main ()
{
struct football player[100];
int option;
//initialize player array
init_player(player);
do
{

//Display menu of options ;
menu();


//Determine users requirements
menu_choice(&option);

//Perform users specified option
switch(option)
{
case 1: //Enter information for all players
player_input(player);
break;
case 2: //display table for all players
goal_table(player);
break;
case 3: //update specific information for a player
goal_update(player);
break;
case 4: //exit from program
break;
default://illegal input
printf ("\n\n This is not an available option\nAvailable options are
1,2,3,4");
}
} while (option !=4);
}

init_player(player)
//------------------
struct football player[];
{
int i,j;
for (i=0;i<100;++i)
{
for (j=0;j<30;++j)
{
player.name[j]=' ';
player.team[j]=' ';
}
player.goals=0;
}
}

menu()
//------------------
{
system ("clear");
printf("\n\n football system ");
printf("\n ------------------- ");
printf("\n\n 1- Enter information ");
printf("\n\n 2- Display table for all players");
printf("\n\n 3- Update specific information");
printf("\n\n 4- Exit from program ");
}


menu_choice(opt)
//------------------
int *opt;
{
printf("\n\n Select one of the above (1-4) : ");
scanf("%d",opt);

//clear screen
system("clear");
}


player_input(player)
//------------------
struct football player[];
{
int i;
char more;
i=-1;
do
{
++i;

//input players name
printf("Enter players name : ");
input_string(player.name);


//input players team
printf("Enter players team : ");
input_string(player.team);


//input number of goals scored
fflush(stdin);
printf("Enter number of goals scored: ");
scanf("%d",&player.goals);

printf("\n %-30s %-30s %4d\n",player.name,player.team,player.goals);
printf("\n %d I Value inside insert\n ",i);
//more players
if (i<99)
{
printf("More playersd to be entered (y/n)---->");
scanf("%s",&more);
}
} while (more == 'y' && i<99);

//Terminate players list
if (i<99) player[i+1].goals=-1;
}


input_string(alpha)
//------------------
char *alpha;
{
int i;
i=-1;

//Flush the keyboard buffer
fflush(stdin);
do
{
++i;

//input a character
alpha=getchar();
} while (alpha !='\n' && i<29);

//Terminate string
alpha='\0';
}

goal_table(player) //Display table of goals scored
//------------------
struct football player[];
{
int i=0;
char cont;

//Output table headings
printf("\n\n Name Team Goals");
printf("\n\n ---- ---- -----");

do
{
//Output player information
printf("\n %d I Value inside display ",i);
printf("\n %-30s %-30s %4d\n",player.name,player.team,player.goals);
++i;
} while (i<=2);
printf("\n\n Press C to continue ");
scanf("%s",&cont);
}

goal_update(player) //update table of goals scored
//------------------
struct football player[];
{
char name[30];
int i,match,goal;

//input players name to be updated
printf("Enter name of player");
input_string(name);

//Find players record
i=0;
while ((player.goals!=-1)&&(i<100)&&(match=strcmp(name,player.name)
!=0))
++i;

//Input number of goals to be added
printf("Enter number of goals to be added to players account");
scanf("%d",&goal);

//Update players record
if (match==0)
player.goals=player.goals+goal;
else
printf("\n\n Player %s is not in the goal table \n",name);
}
 
L

Leo Custodio

As an advice, you should try to indent the code so it would be easy to read.
Also, there is not only one error on that code, but as my compiler shows, 2
pages of them. (GCC 3.3.2). You're not trying to run it under a C++
compiler, are you?

Hint: review it. ;)

Leo Custodio
(e-mail address removed)
--
-----BEGIN PGP PUBLIC KEY BLOCK-----
Version: 2.6.2

mQCNAz/nyswAAAEEAM1Jl14YqNlrUGmr4vh5OKGbDg5qiFnY/Ioqa5j5j9jlTsiH
7EJNlhIvu5OV223D0REUmWbFaKBQlnZAaDRRROb52YPuZ8NQfyu/C5zvTz8qubEx
jWn+nYryqKZxQsDwjntkNIMxx5n+QB7WhDltenCFE/VxYhsTa59EWqUqkz/RAAUR
tC5MZW9uYXJkbyBDLiBDdXN0b2RpbyA8YWxpZW5zcHJpdGVAaG90bWFpbC5jb20+
=xAh5
-----END PGP PUBLIC KEY BLOCK-----

I wrote that example from a book and there is en error in the display
module that it does not showing all the records are entered in the
input module.
I traced with some printf statments without getting the solution . I
think the error in the display module loop condition.
Thanks
Click to expand...
(skip...)
 
R

Richard Heathfield

[Followups set to acllcc++]

[Because you crossposted your article to (among other places) comp.lang.c,
this reply assumes you are programming in C.]

#include <stdio.h>
//-exercise 7.5.1 - Goal Statistics
Click to expand...

If you have a C99 compiler, // comments are legal. If you have a C90
compiler, they are not.
struct football
{
char name[30];
char team[30];
int goals;
};
main ()
Click to expand...

If you have a C90 compiler, implicit int return type is legal. If you have a
C99 compiler, it is not. So, either way, you have a syntax problem.

//input number of goals scored
fflush(stdin);
Click to expand...

The behaviour of fflush on input streams is undefined.
printf("Enter number of goals scored: ");
scanf("%d",&player.goals);
Click to expand...


What if the user types something that isn't a number?
printf("\n %-30s %-30s
%4d\n",player.name,player.team,player.goals);
printf("\n %d I Value inside insert\n ",i);
//more players
if (i<99)
{
printf("More playersd to be entered (y/n)---->");
scanf("%s",&more);
Click to expand...


The 'more' is a single character. %s expects a pointer to an /array/ of
characters. You meant %c, not %s. In any event, an unadorned %s is unwise
(because of buffer overruns).

Fix those problems, and re-test. This will uncover more problems, but should
also lead to greater understanding.

Please pick your newsgroups more carefully next time. Thanks. (I think you
are best off in alt.comp.lang.learn.c-c++ at present, but comp.lang.c is a
decent alternative in this case. The other groups to which you posted are
really implementation groups, not language groups.)
 
D

darklight

i am a begginer a list of errors
i got this when i compile using
gcc -Wall -ggdb filename.c -o filename
play1.c:10: warning: return type defaults to `int'
play1.c: In function `main':
play1.c:14: warning: implicit declaration of function `init_player'
play1.c:19: warning: implicit declaration of function `menu'
play1.c:23: warning: implicit declaration of function `menu_choice'
play1.c:29: warning: implicit declaration of function `player_input'
play1.c:32: warning: implicit declaration of function `goal_table'
play1.c:35: warning: implicit declaration of function `goal_update'
play1.c:43: warning: control reaches end of non-void function
play1.c: At top level:
play1.c:47: warning: return type defaults to `int'
play1.c: In function `init_player':
play1.c:59: warning: control reaches end of non-void function
play1.c: At top level:
play1.c:63: warning: return type defaults to `int'
play1.c: In function `menu':
play1.c:64: warning: implicit declaration of function `system'
play1.c:71: warning: control reaches end of non-void function
play1.c: At top level:
play1.c:76: warning: return type defaults to `int'
play1.c: In function `menu_choice':
play1.c:83: warning: control reaches end of non-void function
play1.c: At top level:
play1.c:88: warning: return type defaults to `int'
play1.c: In function `player_input':
play1.c:126: warning: control reaches end of non-void function
play1.c: At top level:
play1.c:131: warning: return type defaults to `int'
play1.c: In function `input_string':
play1.c:148: warning: control reaches end of non-void function
play1.c: At top level:
play1.c:152: warning: return type defaults to `int'
play1.c: In function `goal_table':
play1.c:170: warning: control reaches end of non-void function
play1.c: At top level:
play1.c:174: warning: return type defaults to `int'
play1.c: In function `goal_update':
play1.c:185: warning: implicit declaration of function `strcmp'
play1.c:198: warning: control reaches end of non-void function
 
A

Alan Coopersmith

(e-mail address removed) ([email protected]) writes in comp.unix.solaris:
|I wrote that example from a book and there is en error in the display
|module that it does not showing all the records are entered in the
|input module.
|I traced with some printf statments without getting the solution . I
|think the error in the display module loop condition.

Could that be because you limit the display to only the first two
records in the display module?
| } while (i<=2);

That seems incredibly obvious, especially since you identified that
as the possible problem.

|init_player(player)
|//------------------
|struct football player[];


If you're just learning C now, get a new book that teaches you ANSI C
(aka C89) and not the ancient K&R style which is long obsolete and will
only cause you problems with modern compilers.

| for (j=0;j<30;++j)
| {
| player.name[j]=' ';
| player.team[j]=' ';
| }

There's no '\0' at the end of those strings, so attempts to print them
may include random garbage at the end, or anything else. It would be
much better to simply do:
player.name[0] = '\0';
player.team[0] = '\0';

Or use memset() if you want to clear the entire string length to 0.

|do
|{
| ++i;
|
| //input a character
| alpha=getchar();
|} while (alpha !='\n' && i<29);

This could be written more simply with fgets()
 
B

Barry Schwarz

I wrote that example from a book and there is en error in the display
Click to expand...

Assuming you copied it correctly, the book has several errors. Any
chance it was written by Schildt?
module that it does not showing all the records are entered in the
input module.
I traced with some printf statments without getting the solution . I
think the error in the display module loop condition.
Thanks



#include <stdio.h>
//-exercise 7.5.1 - Goal Statistics
struct football
{
char name[30];
char team[30];
int goals;
};
main ()
Click to expand...

While older compilers still accept this, the new language standard has
done away with implied return types for functions. Get in the habit
of doing it right:
int main(void)
{
struct football player[100];
int option;
//initialize player array
Click to expand...

Interesting that your book uses a comment style allowed only in C99
yet uses other features (such as above) that are disallowed in C99.
init_player(player);
do
{

//Display menu of options ;
menu();


//Determine users requirements
menu_choice(&option);

//Perform users specified option
switch(option)
{
case 1: //Enter information for all players
player_input(player);
break;
case 2: //display table for all players
goal_table(player);
break;
case 3: //update specific information for a player
goal_update(player);
break;
case 4: //exit from program
break;
default://illegal input
printf ("\n\n This is not an available option\nAvailable options are
1,2,3,4");
Click to expand...

Any time your interactive output does not end with a '\n', you run the
risk of it not being displayed to the user due to buffering
considerations.
}
} while (option !=4);
Click to expand...

Not an error but most recommend an explicit return from main().
}

init_player(player)
//------------------
struct football player[];
Click to expand...

Since this is not main and since you don't return a value, the implied
return type of int here is not acceptable. You must specify void.

While still legal, this construction is very obsolete. (When was the
book written?) The "modern" construction combines the two lines to
void init_player(struct football player[])

This has the additional benefit of allowing you to place function
prototypes in scope before you call the function. I believe this is a
requirement in the new language standard but is a good idea even if
not since it allows the compiler to check that your arguments to the
function have the correct type.
{
int i,j;
for (i=0;i<100;++i)
{
for (j=0;j<30;++j)
{
player.name[j]=' ';
player.team[j]=' ';
}
Click to expand...


You do realize that this serves no purpose?
player.goals=0;
Click to expand...


Neither does this.
}
}

menu()
//------------------
{
system ("clear");
Click to expand...

system is declared in stdlib.h which you did not #include.
printf("\n\n football system ");
printf("\n ------------------- ");
printf("\n\n 1- Enter information ");
printf("\n\n 2- Display table for all players");
printf("\n\n 3- Update specific information");
printf("\n\n 4- Exit from program ");
}


menu_choice(opt)
//------------------
int *opt;
{
printf("\n\n Select one of the above (1-4) : ");
scanf("%d",opt);

//clear screen
system("clear");
Click to expand...

Why on earth would you want to clear the screen right after accepting
user input?
}


player_input(player)
//------------------
struct football player[];
{
int i;
char more;
i=-1;
do
{
++i;

//input players name
printf("Enter players name : ");
input_string(player.name);


//input players team
printf("Enter players team : ");
input_string(player.team);


//input number of goals scored
fflush(stdin);
Click to expand...


This invokes undefined behavior. fflush is defined for output streams
only and has never been defined for input streams.
printf("Enter number of goals scored: ");
scanf("%d",&player.goals);

printf("\n %-30s %-30s %4d\n",player.name,player.team,player.goals);
printf("\n %d I Value inside insert\n ",i);
//more players
if (i<99)
{
printf("More playersd to be entered (y/n)---->");
scanf("%s",&more);
Click to expand...


This is a major problem. more is a single char. %s will accept a
string which is guaranteed to be longer than one char. This invokes
undefined behavior and overwrites whatever is in memory following
more. One possible solution is to use %c.
}
} while (more == 'y' && i<99);
Click to expand...

What if the user types 'Y'?
//Terminate players list
if (i<99) player[i+1].goals=-1;
}


input_string(alpha)
//------------------
char *alpha;
{
int i;
i=-1;

//Flush the keyboard buffer
fflush(stdin);
do
{
++i;

//input a character
alpha=getchar();
} while (alpha !='\n' && i<29);
Click to expand...


When i is 28, the while is true and you loop one final time to accept
alpha[29] ...
//Terminate string
alpha='\0';
Click to expand...


and you then overlay alpha[29] with a nul. It is bad form to destroy
a user's input without telling him. If you coded 28 in the while, he
would at least know that you didn't let him enter the last character.
}

goal_table(player) //Display table of goals scored
//------------------
struct football player[];
{
int i=0;
char cont;

//Output table headings
printf("\n\n Name Team Goals");
printf("\n\n ---- ---- -----");
Click to expand...

These titles and underscores are not wide enough for the 30 character
entries that follow.
do
{
//Output player information
printf("\n %d I Value inside display ",i);
printf("\n %-30s %-30s %4d\n",player.name,player.team,player.goals);
++i;
} while (i<=2);
Click to expand...


This is the source of the error you asked about. Did you mean 99
here? 2 makes no sense at all. This looks like a transcription
error.

Even with 99, there is a logic error. In init_player(), you
initialize name and team to blanks with no terminating '\0'. In
player_input(), you allow for the possibility of less than 100
players. This loop does not test for this and will attempt to print
names and teams that are not strings with the %s format. This invokes
undefined behavior.
printf("\n\n Press C to continue ");
scanf("%s",&cont);
Click to expand...

Another attempt to read a string into a single char. Is this really
what the book says?
}

goal_update(player) //update table of goals scored
//------------------
struct football player[];
{
char name[30];
int i,match,goal;

//input players name to be updated
printf("Enter name of player");
input_string(name);

//Find players record
i=0;
while ((player.goals!=-1)&&(i<100)&&(match=strcmp(name,player.name)
!=0))
Click to expand...


Even though it produces the correct result in most cases, this is a
poor construct. To see why, let's reformat it so the news reader does
not break up the line. The only changes I am making are removing the
/n> that indicates quoted material, inserting a space before each &&,
and inserting a \n after each &&.
while ((player.goals!=-1) &&
(i<100) &&
(match=strcmp(name,player.name)!=0))

The first problem is the second test needs to be first. You cannot
check player.goals if i is 100 or more. That variable does not
exist and attempting to do so invokes undefined behavior. The &&
operator has a short-circuit evaluation so if you rearrange the tests
and the first one fails, the remaining are never evaluated.

Additionally, last expression is "broken." Here it is again with some
addition (and suggestive) white space.
(match = strcmp(name,player.name) != 0)

Since != has higher precedence than =, this expression is evaluated as
( match = (strcmp(name,player.name) != 0) )
which means "assign to match the value 1 or 0 depending on whether the
return value from strcmp is different from or the same as 0,
respectively."

The recommended construction is
( (match = strcmp(name,player.name)) != 0 )
which means "assign the return value from strcmp to match and then
determine if it is different from or the same as 0."

The reason yours works in this case is because you are only interested
in equality or inequality and match will be 0 if and only if strcmp
returns 0. However, for example, if your array were sorted, you would
be interested in two of the three possible return values from strcmp.
Your construction would not give you that. Both negative and positive
returns from strcmp would be combined into a single value for match.
++i;

//Input number of goals to be added
printf("Enter number of goals to be added to players account");
scanf("%d",&goal);
Click to expand...

Don't you think this would be better inside the following if? Why ask
for input if you know you cannot use it?
//Update players record
if (match==0)
player.goals=player.goals+goal;
else
printf("\n\n Player %s is not in the goal table \n",name);
}
Click to expand...


If you give us the name of the book and author and date, we can add it
to the list of books not to use.


<<Remove the del for email>>
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads

Using Do-While loop statement for a selection Menu 3
Need help in debugging tic tac toe (Beginner) 0
Scanf is being prioritized over printf ? 1
Need help in debugging tic tac toe (beginner) 2
Command Line Arguments 0
Help in this program. 2
Tic Tac Toe Game 2
Function is not worked in C 2

Members online

Total: 18 (members: 1, guests: 17)
Robots: 340

Forum statistics

Threads
473,755
Messages
2,569,536
Members
45,015
Latest member
AmbrosePal

Latest Threads

Top