Pointers

[Wabz]

I am trying to use pointers to create a payroll program. I've used
various functions to do the process, but I seem to be having problems
in calling the functions appropriately so each pointer knows where to
read its data from.
Here's what I have so far...can someone help me figure out what am
doing wrong?
Many thanks.

#include <ctype.h>
#include <stdlib.h>
#include <stdio.h>
#define NUM_EMPL 5
#define OVERTIME_RATE 1.5
#define STD_WORK_WEEK 40

struct employee
{
char Name[20]; /* Employee name*/
long id_number; /* Employee clock number/ID No.*/
float WageRate; /* Employees Wage rate*/
float ClockedHours; /* Hours worked by employee */
float OvertimeHours; /* Overtime hours worked by employee*/
float GrossPay; /* Employees Gross Pay*/
struct employee *next;
};

/
************************************************************************/
/* Function:
CalcOvertimeHours */
/
*
*/
/* Purpose: Calculates the number of overtime hours given the
number*/
/* of hours worked. Overtime hours are the number of
hours */
/* worked after 40
hrs. */
/
*
*/
/* Parameters: ClockedHours- Number of hours worked by
employee */
/
*
*/
/* Returns: The number of overtime
hours. */
/
************************************************************************/

float CalcOvertimeHours ( float ClockedHours)

{

/* Test if hours worked is more than 40hrs, and if so return overtime
hours worked*/
if (ClockedHours > STD_WORK_WEEK)

return (ClockedHours - STD_WORK_WEEK); /*return overtime hours*/
else
return (0.0); /*if no overtime worked return zero*/
}



/
***************************************************************************/
/* Function: CalcGrossPay */
/* Purpose: Calculates gross pay including overtime given the
number */
/* of hours an employee works and wage rate. */
/* */
/*Parameters: ClockedHours - Number of hours worked by employee
*/
/* WageRate - Hourly rates of employees. */
/* GrossPay - Array of gross pay amounts for
employees. */
/* OvertimeHours - Overtime hours worked by employee */
/* */
/*Returns: Gross pay */
/* */
/
****************************************************************************/

float CalcGrossPay (float ClockedHours,float OvertimeHours,float
WageRate)
{

/*Local variable declaration*/

float GrossPay; /* Holds Grosspay calculated*/

/* Calculate GrossPay including overtime pay*/

if (ClockedHours > STD_WORK_WEEK)
{
OvertimeHours = ClockedHours - STD_WORK_WEEK;
GrossPay = WageRate*STD_WORK_WEEK + (OvertimeHours *
(WageRate*OVERTIME_RATE));
return (GrossPay);
}

/*calculate Gross Pay where there is no overtime*/

else
{
GrossPay = WageRate*ClockedHours;
return (GrossPay);
}
}

/
*----------------------------------------------------------------------
*/
/
*
*/
/* FUNCTION:
print_list */
/
*
*/
/* DESCRIPTION: This function will print the contents of a
linked */
/* list. It will traverse the list from beginning to
the */
/* end, printing the contents at each
node. */
/
*
*/
/* PARAMETERS: emp1 - pointer to a linked
list */
/
*
*/
/* OUTPUTS:
None */
/
*
*/
/* CALLS:
None */
/
*
*/
/
*----------------------------------------------------------------------
*/
void print_list(emp1)
struct employee *emp1;
{

/* Declare variables */

FILE *outputfileptr; /* Pointer to the output file */
struct employee *tmp; /* tmp pointer value to current node */
int i = 0; /* counts the nodes printed */


/* open a file called Payroll.txt */
if ((outputfileptr = fopen("Payroll.txt", "w")) == (FILE *)
NULL)
{
fprintf(stderr, "Error, Unable to open file\n");
exit(1);
}


/* Start a beginning of list and print out each value
*/
/* loop until tmp points to null (remember null is 0 or false)
*/
for(tmp = emp1; tmp ; tmp = tmp->next)
{
i++;
/* print out header information to file */
fprintf (outputfileptr, "\nPAYROLL\n\n\n");
fprintf (outputfileptr, "\n
\n--------------------------------------------------------------------
\n");
fprintf (outputfileptr, "\nName \t\tEmployee ID \tWage \tHours \tOT
\tGross Pay\n");
fprintf (outputfileptr,
"------------------------------------------------------------------------
\n");
fprintf(outputfileptr,"%s \t\t%06i \t\t%5.1f \t%5.1f \t%8.2f\n",tmp-

Name, tmp->id_number,tmp->WageRate, tmp->ClockedHours,tmp-
OvertimeHours, tmp->GrossPay );

}

printf("\n\nTotal Number of Employees = %d\n", i);

}



/
*----------------------------------------------------------------------
*/
/
*
*/
/* FUNCTION:
main */
/
*
*/
/* DESCRIPTION: This function will prompt the user for an
employee */
/* id and wage until the user indicates they are
finished.*/
/* At that point, a list of id and wages will
be */
/*
generated. */
/
*
*/
/* PARAMETERS:
None */
/
*
*/
/* OUTPUTS:
None */
/
*
*/
/* CALLS:
print_list */
/
*
*/
/
*----------------------------------------------------------------------
*/
int main ()
{

char answer[80];
int more_data = 1;
char value;
float OvertimeHours; /* Overtime hours worked by employee*/
float ClockedHours; /* Hours worked by employee */
float WageRate; /* Employees Wage rate*/
float GrossPay; /* Employees Gross Pay*/

struct employee *current_ptr, /* pointer to current node */
*head_ptr; /* always points to first node */

/* Set up storage for first node */
head_ptr = (struct employee *) malloc (sizeof(struct employee));
current_ptr = head_ptr;


/* Print program information to the screen*/
printf ("This is a program to calculate gross pay.\n");
printf ("Please enter employee data when prompted.\n\n");

while (more_data)
{
/*printf ("\n");
printf ("Please enter the employee's name: ");

scanf ("\n");
gets ("%s", &current_ptr->Name); CHECK THIS LATER*/


printf("\nEnter employee Name: ");
scanf("%s", &current_ptr->Name);

printf("\nEnter employee ID: ");
scanf("%d", &current_ptr->id_number);

printf("\nEnter Hours Worked: ");
scanf("%f", &current_ptr->ClockedHours);


printf("\nEnter employee hourly wage: ");
scanf("%f", &current_ptr->WageRate);

printf("\n\nWould you like to add another employee? (y/n): ");
scanf("%s", answer);

/* Ask user if they want to add another employee */
if (value = toupper(answer[0]) != 'Y')
{
current_ptr->next = (struct employee *) NULL;
more_data = 0;
}
else
{
/* set the next pointer of the current node to point to the new
node */
current_ptr->next = (struct employee *) malloc
(sizeof(struct employee));
/* move the current node pointer to the new node */
current_ptr = current_ptr->next;
}

}



/* Calc overtime hours Function call */
OvertimeHours = CalcOvertimeHours (ClockedHours);


/* Calc Gross Pay Function call */
GrossPay = CalcGrossPay ( ClockedHours, OvertimeHours, WageRate);

/* print out listing of all employee id's and wages that were
entered */
print_list(head_ptr);

printf("\n\nEnd of program\n");
return 0;
}

 

[Flash Gordon]

Wabz wrote, On 06/08/07 20:12:

I am trying to use pointers to create a payroll program. I've used
various functions to do the process, but I seem to be having problems
in calling the functions appropriately so each pointer knows where to
read its data from.

Only one of your functions uses a pointer.

Here's what I have so far...can someone help me figure out what am
doing wrong?

It would help if you were more specific about your problem. What appears
not to work?

Many thanks.

#include <ctype.h>
#include <stdlib.h>
#include <stdio.h>
#define NUM_EMPL 5
#define OVERTIME_RATE 1.5
#define STD_WORK_WEEK 40

struct employee
{
char Name[20]; /* Employee name*/

That is not much space for a name.

long id_number; /* Employee clock number/ID No.*/
float WageRate; /* Employees Wage rate*/
float ClockedHours; /* Hours worked by employee */
float OvertimeHours; /* Overtime hours worked by employee*/
float GrossPay; /* Employees Gross Pay*/

Why use float? When you want floating point numbers you should almost
always use double rather than float, you should only use float when you
know you have either memory or speed problems that you have verified
will be solved by using float *AND* you have verified float will be
sufficient.

struct employee *next;
};

/
************************************************************************/

Keep your line length down. Yours is so long that the code has been
broken by line wrapping. It will only get worse as things are quoted.

float CalcOvertimeHours ( float ClockedHours)

{

/* Test if hours worked is more than 40hrs, and if so return overtime
hours worked*/
if (ClockedHours > STD_WORK_WEEK)

return (ClockedHours - STD_WORK_WEEK); /*return overtime hours*/

Return is not a function, it does not need the parenthesis.

else
return (0.0); /*if no overtime worked return zero*/
}

float CalcGrossPay (float ClockedHours,float OvertimeHours,float
WageRate)
{

/*Local variable declaration*/

float GrossPay; /* Holds Grosspay calculated*/

/* Calculate GrossPay including overtime pay*/

if (ClockedHours > STD_WORK_WEEK)
{
OvertimeHours = ClockedHours - STD_WORK_WEEK;
GrossPay = WageRate*STD_WORK_WEEK + (OvertimeHours *
(WageRate*OVERTIME_RATE));
return (GrossPay);

Why bother with a local variable when you immediately return it?

}

/*calculate Gross Pay where there is no overtime*/

else
{
GrossPay = WageRate*ClockedHours;
return (GrossPay);
}
}

void print_list(emp1)
struct employee *emp1;
{

Why use the old fashioned K&R style declaration? It hobbles the type
checking.

void print_list(struct employee *empl)
{

/* Declare variables */

FILE *outputfileptr; /* Pointer to the output file */
struct employee *tmp; /* tmp pointer value to current node */
int i = 0; /* counts the nodes printed */


/* open a file called Payroll.txt */
if ((outputfileptr = fopen("Payroll.txt", "w")) == (FILE *)
NULL)
{
fprintf(stderr, "Error, Unable to open file\n");
exit(1);

This is not a portable value to pass to exit, and on at least one system
it would actually mean success!
exit(EXIT_FAILURE);

}


/* Start a beginning of list and print out each value
*/
/* loop until tmp points to null (remember null is 0 or false)
*/
for(tmp = emp1; tmp ; tmp = tmp->next)
{
i++;
/* print out header information to file */
fprintf (outputfileptr, "\nPAYROLL\n\n\n");
fprintf (outputfileptr, "\n
\n--------------------------------------------------------------------
\n");
fprintf (outputfileptr, "\nName \t\tEmployee ID \tWage \tHours \tOT
\tGross Pay\n");
fprintf (outputfileptr,
"------------------------------------------------------------------------
\n");
fprintf(outputfileptr,"%s \t\t%06i \t\t%5.1f \t%5.1f \t%8.2f\n",
tmp->Name, tmp->id_number,tmp->WageRate, tmp->ClockedHours,
tmp->OvertimeHours, tmp->GrossPay );

id_number is a long but you are using he format specifier for an int.
Also you do not have enough format specifiers for the number of things
you are trying to print.

}

printf("\n\nTotal Number of Employees = %d\n", i);

}

int main ()
{

char answer[80];
int more_data = 1;
char value;
float OvertimeHours; /* Overtime hours worked by employee*/
float ClockedHours; /* Hours worked by employee */
float WageRate; /* Employees Wage rate*/
float GrossPay; /* Employees Gross Pay*/

struct employee *current_ptr, /* pointer to current node */
*head_ptr; /* always points to first node */

/* Set up storage for first node */
head_ptr = (struct employee *) malloc (sizeof(struct employee));

Loose the cast, it is not needed and if your compiler complains without
it then it means you have a serious problem that the cast will *not*
fix. Also there are better ways to use sizeof.
head_ptr = malloc (sizeof *head_ptr);

current_ptr = head_ptr;


/* Print program information to the screen*/
printf ("This is a program to calculate gross pay.\n");
printf ("Please enter employee data when prompted.\n\n");

while (more_data)
{
/*printf ("\n");
printf ("Please enter the employee's name: ");

scanf ("\n");
gets ("%s", &current_ptr->Name); CHECK THIS LATER*/


printf("\nEnter employee Name: ");

You need to flush stdout here or the prompt may well not be displayed on
some systems.

scanf("%s", &current_ptr->Name);

Using an unadorned %s with scanf is just as bad as using gets. DO NOT DO
IT. Loose the & as %s expects a pointer to char, NOT a pointer to array
of char. As you don't check the value of scanf you don't know if it has
succeeded.

printf("\nEnter employee ID: ");
scanf("%d", &current_ptr->id_number);

Again, what if scanf fails?

printf("\nEnter Hours Worked: ");
scanf("%f", &current_ptr->ClockedHours);


printf("\nEnter employee hourly wage: ");
scanf("%f", &current_ptr->WageRate);

printf("\n\nWould you like to add another employee? (y/n): ");
scanf("%s", answer);

All the comments about the first scanf apply.

/* Ask user if they want to add another employee */
if (value = toupper(answer[0]) != 'Y')
{
current_ptr->next = (struct employee *) NULL;

Why the cast? It is not needed and just helps make the code less readable.

more_data = 0;
}
else
{
/* set the next pointer of the current node to point to the new
node */
current_ptr->next = (struct employee *) malloc
(sizeof(struct employee));

All the comments about your last malloc call apply here as well.

/* move the current node pointer to the new node */
current_ptr = current_ptr->next;
}

}



/* Calc overtime hours Function call */
OvertimeHours = CalcOvertimeHours (ClockedHours);


/* Calc Gross Pay Function call */
GrossPay = CalcGrossPay ( ClockedHours, OvertimeHours, WageRate);

You have not assigned anything to ClockedHours or WageRate before this
call, only to the similarly named fields in your structure.

You don't assign anything to the OvertimeHours or GrossPay fields in
your structure, only to local variables with similar names.

/* print out listing of all employee id's and wages that were
entered */
print_list(head_ptr);

printf("\n\nEnd of program\n");
return 0;
}

I've pointed out several problems above, please fix *all* of them
together with anything other people point our or ask about anything you
do not understand in the replies you get. If the program still does not
do what you want then please ensure that it is better formatted when
posting it again with your attempts at fixing it, don't use tabs and
keep the line length down.

 

[santosh]

I am trying to use pointers to create a payroll program. I've used
various functions to do the process, but I seem to be having problems
in calling the functions appropriately so each pointer knows where to
read its data from.
Here's what I have so far...can someone help me figure out what am
doing wrong?
Many thanks.

#include <ctype.h>
#include <stdlib.h>
#include <stdio.h>
#define NUM_EMPL 5
#define OVERTIME_RATE 1.5
#define STD_WORK_WEEK 40

struct employee
{
char Name[20]; /* Employee name*/
long id_number; /* Employee clock number/ID No.*/
float WageRate; /* Employees Wage rate*/
float ClockedHours; /* Hours worked by employee */
float OvertimeHours; /* Overtime hours worked by employee*/
float GrossPay; /* Employees Gross Pay*/

I'd declare the float objects above as double. On most architectures,
calculations involving double objects are significantly faster than for
float.

struct employee *next;
};

/
************************************************************************/

A comment in C starts with a '/*' sequence. The above is malformed since
there is a newline character between '/' and '*'.

/* Function:
CalcOvertimeHours */
/
*
*/

Again. Did you try compiling your code?

/* Purpose: Calculates the number of overtime hours given the
number*/
/* of hours worked. Overtime hours are the number of
hours */
/* worked after 40
hrs. */
/
*
*/
/* Parameters: ClockedHours- Number of hours worked by
employee */
/
*
*/
/* Returns: The number of overtime
hours. */
/
************************************************************************/

float CalcOvertimeHours ( float ClockedHours)

{

/* Test if hours worked is more than 40hrs, and if so return overtime
hours worked*/
if (ClockedHours > STD_WORK_WEEK)

Beware comparisons involving floating values can give misleading results.
Google for "Goldberg Floating point" for a classic paper on floating point
precision issues.

return (ClockedHours - STD_WORK_WEEK); /*return overtime hours*/
else
return (0.0); /*if no overtime worked return zero*/
}



/
***************************************************************************/
/* Function: CalcGrossPay */
/* Purpose: Calculates gross pay including overtime given the
number */
/* of hours an employee works and wage rate. */
/* */
/*Parameters: ClockedHours - Number of hours worked by employee
*/
/* WageRate - Hourly rates of employees. */
/* GrossPay - Array of gross pay amounts for
employees. */
/* OvertimeHours - Overtime hours worked by employee */
/* */
/*Returns: Gross pay */
/* */
/
****************************************************************************/

float CalcGrossPay (float ClockedHours,float OvertimeHours,float
WageRate)
{

/*Local variable declaration*/

float GrossPay; /* Holds Grosspay calculated*/

/* Calculate GrossPay including overtime pay*/

if (ClockedHours > STD_WORK_WEEK)
{
OvertimeHours = ClockedHours - STD_WORK_WEEK;

Why not call the previous function instead of repeating code?

GrossPay = WageRate*STD_WORK_WEEK + (OvertimeHours *
(WageRate*OVERTIME_RATE));
return (GrossPay);

The parenthesis are not needed.

}

/*calculate Gross Pay where there is no overtime*/

else
{
GrossPay = WageRate*ClockedHours;
return (GrossPay);
}
}

Don't you have a decent editor to do some readable formatting?

/
*----------------------------------------------------------------------
*/
/
*
*/
/* FUNCTION:
print_list */
/
*
*/
/* DESCRIPTION: This function will print the contents of a
linked */
/* list. It will traverse the list from beginning to
the */
/* end, printing the contents at each
node. */
/
*
*/
/* PARAMETERS: emp1 - pointer to a linked
list */
/
*
*/
/* OUTPUTS:
None */
/
*
*/
/* CALLS:
None */
/
*
*/
/
*----------------------------------------------------------------------
*/
void print_list(emp1)
struct employee *emp1;

Mixing ISO type and K&R type function declarations is not a very good idea.
There's no reason to use the old-style function declarations for new code.
Prototypes help the compiler to do more error checking for you.

{

/* Declare variables */

FILE *outputfileptr; /* Pointer to the output file */
struct employee *tmp; /* tmp pointer value to current node */
int i = 0; /* counts the nodes printed */


/* open a file called Payroll.txt */
if ((outputfileptr = fopen("Payroll.txt", "w")) == (FILE *)
NULL)

The cast to FILE * is not needed.

{
fprintf(stderr, "Error, Unable to open file\n");
exit(1);

An exit status of one is not portable. Portable values are 0 and
EXIT_SUCCESS for normal termination and EXIT_FAILURE for abnormal ones.

}


/* Start a beginning of list and print out each value
*/
/* loop until tmp points to null (remember null is 0 or false)
*/
for(tmp = emp1; tmp ; tmp = tmp->next)
{
i++;
/* print out header information to file */
fprintf (outputfileptr, "\nPAYROLL\n\n\n");
fprintf (outputfileptr, "\n
\n--------------------------------------------------------------------
\n");

You can't embed newlines in format strings directly. Rather split the format
string to one or more adjacent string literals, which are automatically
concatenated, and include newlines in the form of an '\n' escape sequence.

fprintf (outputfileptr, "\nName \t\tEmployee ID \tWage \tHours \tOT
\tGross Pay\n");
fprintf (outputfileptr,
"------------------------------------------------------------------------
\n");

Same as above.

fprintf(outputfileptr,"%s \t\t%06i \t\t%5.1f \t%5.1f \t%8.2f\n",tmp-

Your id_number member is declared as type long. The 'i' format specifier is
for plain int. To print longs use 'li', 'ld' or 'lu' specifiers.

Also you have an excess argument, namely tmp->GrossPay. You've invoked
undefined behaviour. The number and type of format specifiers and the
corresponding arguments must match.

}

printf("\n\nTotal Number of Employees = %d\n", i);

}



/
*----------------------------------------------------------------------
*/
/
*
*/
/* FUNCTION:
main */
/
*
*/
/* DESCRIPTION: This function will prompt the user for an
employee */
/* id and wage until the user indicates they are
finished.*/
/* At that point, a list of id and wages will
be */
/*
generated. */
/
*
*/
/* PARAMETERS:
None */
/
*
*/
/* OUTPUTS:
None */
/
*
*/
/* CALLS:
print_list */
/
*
*/
/
*----------------------------------------------------------------------
*/
int main ()
{

char answer[80];
int more_data = 1;
char value;
float OvertimeHours; /* Overtime hours worked by employee*/
float ClockedHours; /* Hours worked by employee */
float WageRate; /* Employees Wage rate*/
float GrossPay; /* Employees Gross Pay*/

struct employee *current_ptr, /* pointer to current node */
*head_ptr; /* always points to first node */

/* Set up storage for first node */
head_ptr = (struct employee *) malloc (sizeof(struct employee));

Don't cast the return value of *alloc functions in C.

head_ptr = malloc(sizeof *head_ptr);

current_ptr = head_ptr;

/* Print program information to the screen*/
printf ("This is a program to calculate gross pay.\n");
printf ("Please enter employee data when prompted.\n\n");

while (more_data)
{
/*printf ("\n");
printf ("Please enter the employee's name: ");

scanf ("\n");
gets ("%s", &current_ptr->Name); CHECK THIS LATER*/


printf("\nEnter employee Name: ");

Unless output is terminated by a newline, it's not guaranteed that it'll be
written to the device synchronously.

scanf("%s", &current_ptr->Name);

Almost as dangerous as gets. A name length over 20 characters will overflow
buffer limits.

Take in input with fgets and use sscanf to convert them.

printf("\nEnter employee ID: ");
scanf("%d", &current_ptr->id_number);

Use 'ld' to convert to a long value.

printf("\nEnter Hours Worked: ");
scanf("%f", &current_ptr->ClockedHours);


printf("\nEnter employee hourly wage: ");
scanf("%f", &current_ptr->WageRate);

printf("\n\nWould you like to add another employee? (y/n): ");
scanf("%s", answer);

An array of 80 to receive a single char?

/* Ask user if they want to add another employee */
if (value = toupper(answer[0]) != 'Y')

Declare value as an int, since toupper and tolower return an int.

Also you might want to do:

if ((value = toupper(answer[0])) != 'Y')

though your form will work, it might not be exactly what you meant to write.

{
current_ptr->next = (struct employee *) NULL;
more_data = 0;
}
else
{
/* set the next pointer of the current node to point to the new
node */
current_ptr->next = (struct employee *) malloc
(sizeof(struct employee));
/* move the current node pointer to the new node */
current_ptr = current_ptr->next;
}

}



/* Calc overtime hours Function call */
OvertimeHours = CalcOvertimeHours (ClockedHours);


/* Calc Gross Pay Function call */
GrossPay = CalcGrossPay ( ClockedHours, OvertimeHours, WageRate);

/* print out listing of all employee id's and wages that were
entered */
print_list(head_ptr);

printf("\n\nEnd of program\n");
return 0;
}

Try actually compiling the code and observing the compiler errors.
Set your compiler to strictly conform to ISO C and enable maximum diagnostic
level. For gcc, this can be done by the command-line:

gcc -Wall -Wextra -ansi -pedantic -O2 -c input.c

 

[Army1987]

I am trying to use pointers to create a payroll program. I've used
various functions to do the process, but I seem to be having problems
in calling the functions appropriately so each pointer knows where to
read its data from.
Here's what I have so far...can someone help me figure out what am
doing wrong?
Many thanks.

#include <ctype.h>
#include <stdlib.h>
#include <stdio.h>
#define NUM_EMPL 5
#define OVERTIME_RATE 1.5
#define STD_WORK_WEEK 40 To be consistent I'd use 40.0.

struct employee
{
char Name[20]; /* Employee name*/
long id_number; /* Employee clock number/ID No.*/
float WageRate; /* Employees Wage rate*/
float ClockedHours; /* Hours worked by employee */
float OvertimeHours; /* Overtime hours worked by employee*/
float GrossPay; /* Employees Gross Pay*/

I'd declare the float objects above as double. On most architectures,
calculations involving double objects are significantly faster than for
float.

Yes, but if he's using a linked list odds are he can need umpteen
such structs, and space might be important. (I'd simply use an
integral type encoding wages in cents and times in minutes or
seconds, so I'd avoid rounding errors at all.)
[snip]

An exit status of one is not portable. Portable values are 0 and
EXIT_SUCCESS for normal termination and EXIT_FAILURE for abnormal ones.

exit(EXIT_FAILURE) causes normal termination with unsuccessful
status, not abnormal termination. abort() causes abnormal
termination. grep exits with EXIT_FAILURE when it can read files
without problems but doesn't find any match to the provided regex.
splint exit with EXIT_FAILURE if it finds more than the number
specified by -expect (0 if there is no -expect option). If I know
that the_thing_that_should_not_be is false, but it would cause big
problems if true, I can write assert(!the_thing_that_should_not_be)
which will call abort() if I was wrong, so that I can check what
was wrong in my code. They're different concepts.
[snip]

fprintf(outputfileptr,"%s \t\t%06i \t\t%5.1f \t%5.1f \t%8.2f\n",tmp-

Also you have an excess argument, namely tmp->GrossPay. You've invoked
undefined behaviour.

7.19.6.1p2 seems to disagree with you. (Not that I think it is
ever a good thing to do...
[snip]

Try actually compiling the code and observing the compiler errors.
Set your compiler to strictly conform to ISO C and enable maximum
diagnostic level. For gcc, this can be done by the command-line:

gcc -Wall -Wextra -ansi -pedantic -O2 -c input.c

Any reasons for -c, why not call the linker for a complete
program?

 

[Wabz]

Hello good people.

Thanks for all your comments on the previous code. I implemented some
changes (not all but some) I figure I need to first get the program to
do what i want it to do.

Am facing a few issues:

-When executed, the program only displyas calculated overtime for the
last entered employee. How do i get it to retain all values entered
and not just the last value?
-The Gross pay function is not displaying any results/output. I have
checked and rechecked it but can't seem to find what is missing.

The program compiles just fine and has no errors though. It just is
not doing all of what it's supposed to do.

Once again many thanks for your valued comments.



#include <ctype.h>
#include <stdlib.h>
#include <stdio.h>
#define NUM_EMPL 5
#define OVERTIME_RATE 1.5
#define STD_WORK_WEEK 40

struct employee
{
char Name[50]; /* Employee name*/
long id_number; /* Employee clock number/ID No.*/
float WageRate; /* Employees Wage rate*/
float ClockedHours; /* Hours worked by employee */
float OvertimeHours; /* Overtime hours worked by employee*/
float GrossPay; /* Employees Gross Pay*/
struct employee *next; /*Structure pointer*/
};


/*Function to Calculate OvertimeHours*/

float CalcOvertimeHours ( float ClockedHours)
{
/* Test if hours worked is more than 40hrs*/
/* and if so return overtime hours worked*/
if (ClockedHours > STD_WORK_WEEK)

return (ClockedHours - STD_WORK_WEEK);
else
return (0.0);
}

/*Function to Calculate GrossPay*/
float CalcGrossPay (float ClockedHours,float OvertimeHours,float
WageRate)

{
/*Local variable declaration*/
float GrossPay; /* Holds Grosspay calculated*/

/* Calculate GrossPay including overtime pay*/
if (ClockedHours > STD_WORK_WEEK)
{
OvertimeHours = ClockedHours - STD_WORK_WEEK;
GrossPay = WageRate*STD_WORK_WEEK + (OvertimeHours *
(WageRate*OVERTIME_RATE));
return (GrossPay);
}
/*calculate Gross Pay where there is no overtime*/
else
{
GrossPay = WageRate*ClockedHours;
return (GrossPay);
}
}

/* FUNCTION to print_list*/

void print_list(emp1)
struct employee *emp1;
{
/* Declare variables */

FILE *outputfileptr; /* Pointer to the output file */
struct employee *tmp; /* tmp pointer value to current node */
int i = 0; /* counts the nodes printed */


/* open a file called Payroll.txt */
if ((outputfileptr = fopen("Payroll.txt", "w")) == (FILE *)
NULL)
{
fprintf(stderr, "Error, Unable to open file\n");
exit(1);
}

/* print out header information to file */
fprintf (outputfileptr, "\nPAYROLL\n\n\n");
fprintf (outputfileptr, "\n
\n--------------------------------------------------------------------
\n");
fprintf (outputfileptr, "\nName \t\tEmployee ID \tWage \tHours \tOT \t
\tGross Pay\n");
fprintf (outputfileptr,
"------------------------------------------------------------------------
\n");

/* Start a beginning of list and print out each value */
/* loop until tmp points to null (remember null is 0 or false) */
for(tmp = emp1; tmp ; tmp = tmp->next)
{
i++;

fprintf(outputfileptr,"%s \t\t%06li \t\t%5.1f \t%5.1f \t%8.2f\n",tmp-

Name, tmp->id_number,tmp->WageRate, tmp->ClockedHours,tmp-
OvertimeHours, tmp->GrossPay );

}
printf("\n\nTotal Number of Employees = %d\n", i);
}

int main ()
{

char answer[80];
int more_data = 1;
char value;
struct employee *emp1;
struct employee *tmp; /* tmp pointer value to current node */
int i=0;

struct employee *current_ptr, /* pointer to current node */
*head_ptr; /* always points to first node */

/* Set up storage for first node */
head_ptr = (struct employee *) malloc (sizeof(struct employee));
current_ptr = head_ptr;

/* Print program information to the screen*/
printf ("This is a program to calculate gross pay.\n");
printf ("Please enter employee data when prompted.\n\n");

while (more_data)
{

printf("\nEnter employee Name: ");
scanf("%s", &current_ptr->Name);

printf("\nEnter employee ID: ");
scanf("%d", &current_ptr->id_number);

printf("\nEnter Hours Worked: ");
scanf("%f", &current_ptr->ClockedHours);


printf("\nEnter employee hourly wage: ");
scanf("%f", &current_ptr->WageRate);

printf("\n\nWould you like to add another employee? (y/n): ");
scanf("%s", answer);

/* Ask user if they want to add another employee */
if (value = toupper(answer[0]) != 'Y')
{
current_ptr->next = (struct employee *) NULL;
more_data = 0;
}
else
{
/* set the next pointer of the current node to point to the new node
*/
current_ptr->next = (struct employee *) malloc (sizeof(struct
employee));
/* move the current node pointer to the new node */
current_ptr = current_ptr->next;
}

}

/* Function calls*/
for(tmp = emp1; tmp ; tmp = tmp->next)
{
i++;
/* Calc overtime hours Function call */
current_ptr->OvertimeHours = CalcOvertimeHours (current_ptr-

ClockedHours);

/* Calc Gross Pay Function call */
current_ptr->GrossPay = CalcGrossPay ( current_ptr->ClockedHours,
current_ptr->OvertimeHours, current_ptr->WageRate);

/* Print out listing of all employee id's and wages that were entered
*/
print_list(head_ptr);

printf("\n\nEnd of program\n");
return 0;

}
}

 

[Flash Gordon]

Wabz wrote, On 07/08/07 12:53:

Hello good people.

Thanks for all your comments on the previous code. I implemented some
changes (not all but some) I figure I need to first get the program to
do what i want it to do.

Well, you are wrong. First you need to fix all the problems you know
about, then you can look and see what the situation is.

Am facing a few issues:

-When executed, the program only displyas calculated overtime for the
last entered employee. How do i get it to retain all values entered
and not just the last value?
-The Gross pay function is not displaying any results/output. I have
checked and rechecked it but can't seem to find what is missing.

Check what goes on in the calling code, of course!

The program compiles just fine and has no errors though. It just is
not doing all of what it's supposed to do.

Turn up your warning level on the compiler, it can almost certainly tell
you a few more things you are doing wrong. If using gcc I would suggest
you start with "-ansi -pedantic -Wall -Wextra -O" which will, amongst
other things, tell you that emp1 is being used without ever having been
assigned a value. Also look at what is in your last for loop and what is
not. Most importantly, do something about your indenting, this time the
code reached me with no indenting at all possibly due to you using tabs.
Complete lack of indenting makes it extremely hard to read. Indent with
spaces.

 

[Ben Bacarisse]

Hello good people.

Thanks for all your comments on the previous code. I implemented some
changes (not all but some) I figure I need to first get the program to
do what i want it to do.

It is never wise to leave something that know to be wrong. if you are
very experienced, you might be pretty sure that you can leave it for
now and deal with other things, but if you are a beginner, how can you
know what is critical and what is not?

Am facing a few issues:

-When executed, the program only displyas calculated overtime for the
last entered employee. How do i get it to retain all values entered
and not just the last value?

That is only one symptom. You program has quite a few errors. The
reason why it does what you say is actually self-evident in the code,
but it could do almost anything because it exhibits the dreaded
"undefined behaviour" before getting to that part.

for(tmp = emp1; tmp ; tmp = tmp->next)
BOOM!

{
i++;
/* Calc overtime hours Function call */
current_ptr->OvertimeHours = CalcOvertimeHours (current_ptr-

emp1 is not initialised so this loop's behaviour is undefined.

The problem you are worried about (much less serious) is that you
calculate the gross pay on only one element: current_ptr (which is
still pointing to the last element of the list.

Please don't just fix this and ignore the other problems. My second
most serious concern is 'scanf("%s", answer)'. If you don't know why
this is Very Bad Indeed, think about what happens when the user falls
asleep on the Y key: yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy
yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy
yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy
yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy
yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy...

 

[santosh]

Hello good people.

Thanks for all your comments on the previous code. I implemented some
changes (not all but some) I figure I need to first get the program to
do what i want it to do.

The program won't perform correctly until and unless you implement all the
changes we have suggested. Even after that, there may be more corrections
possible.

The program compiles just fine and has no errors though. It just is
not doing all of what it's supposed to do.

After fixing all sorts of formatting errors, your code compiles with the
following warnings from gcc here:

$ gcc -Wall -Wextra -ansi -pedantic -O2 -o test test.c
0016.c: In function ?print_list?:
0016.c:94: warning: too many arguments for format
0016.c: In function ?main?:
0016.c:125: warning: format ?%s? expects type ?char *?, but argument 2 has
type ?char (*)[49u]?
0016.c:128: warning: format ?%d? expects type ?int *?, but argument 2 has
type ?long int *?
0016.c:141: warning: suggest parentheses around assignment used as truth
value
0016.c:158: warning: ?emp1? is used uninitialized in this function
0016.c:176: warning: control reaches end of non-void function

The fixes for all the above issues were given to you in the previous posts.
Why didn't you implement them?

Once again many thanks for your valued comments.

They have no value until you try them out.

<snip same code>

 

[CBFalconer]

.... snip ...

That is only one symptom. You program has quite a few errors.
The reason why it does what you say is actually self-evident in
the code, but it could do almost anything because it exhibits the
dreaded "undefined behaviour" before getting to that part.



BOOM!

Now what can you find wrong with that?

 

[santosh]

Now what can you find wrong with that?

emp1 is used uninitialised.

 

[Ben Bacarisse]

Now what can you find wrong with that?

emp1 was not initialised. Just read a few lines further. Not the
best layout, for sure, but I was not pulling any punches.

 

[Spoon]

emp1 was not initialised. Just read a few lines further. Not the
best layout, for sure, but I was not pulling any punches.

I'm confused.

void print_list(emp1)
struct employee *emp1;
{
/*** emp1 is a parameter, right? ***/
....
for(tmp = emp1; tmp ; tmp = tmp->next)

initialize tmp to emp1

What's wrong with that?

 

[osmium]

I'm confused.

void print_list(emp1)
struct employee *emp1;
{
/*** emp1 is a parameter, right? ***/
...
for(tmp = emp1; tmp ; tmp = tmp->next)

initialize tmp to emp1

What's wrong with that?

There's nothing wrong with that. I think the point is that emp1 was not
initialized, which, in itself, is OK too, but this statement makes it
explicit that the non-initialized value will actually be *used*. Kind of
the final nail in a coffin kind of thing.

 

[Spoon]

There's nothing wrong with that. I think the point is that emp1 was not
initialized, which, in itself, is OK too, but this statement makes it
explicit that the non-initialized value will actually be *used*. Kind of
the final nail in a coffin kind of thing.

I don't understand.
emp1 is a function parameter.
Therefore it be initialized when the function is invoked?

 

[Stephen Sprunk]

There's nothing wrong with that. I think the point is that emp1 was not
initialized, which, in itself, is OK too, but this statement makes it
explicit that the non-initialized value will actually be *used*. Kind of
the final nail in a coffin kind of thing.

emp1 _is_ initialized; it's an argument to the function. You might be
missing that because the OP is using an obsolete K&R style argument.

S

 

[Ben Bacarisse]

I'm confused.

So I see!

void print_list(emp1)
struct employee *emp1;
{
/*** emp1 is a parameter, right? ***/
...
for(tmp = emp1; tmp ; tmp = tmp->next)

initialize tmp to emp1

What's wrong with that?

Nothing. I did not comment on that. Exactly the same code (no doubt
cut and pasted) appears in main(). There, emp1 is not initialized.
The result is undefined behaviour.

One simple correction requires justification twice!

 

[Ben Bacarisse]

emp1 _is_ initialized; it's an argument to the function. You might be
missing that because the OP is using an obsolete K&R style argument.

If you two are going to carry on talking about this code, please note
that it has nothing to do with the code I commented on. The code in
print_list if fine (from this point of view).