How can I make this simple assignment more eloquent ?

**gordonfreeman** · 03-30-2012

Hey guys I'm new to this forum and I'd like to find out if someone out there can make the code I've written for this homework assignment more eloquent. I somehow feel that the code is just really poorly written and my main goal is to reduce the number of lines of code I have written while still providing the same functionality.

*THIS IS A HOMEWORK ASSIGNMENT*
The assignment states the following:

Write a C program to calculate salary raise for employees.
If salary is between 0 < 30000 rate is 7.0%
If salary is between 30000 <= 40000 the rate is 5.5%
If salary is greater than 40000 the rate is 4.0%

1. you are to use appropriate functions and pass parameters for this program. Please include at least one function that uses the return statement and at least one function that does not use the return statement.
2. Draw a structure chart that shows all the functions.
3. Write a function to allow user to inpu the salaries only.
4. Write a function to calculate the raises, the new salaries(raise+salary), total of the original salaries, the total of raises and the total of all the new salaries with the raises.
5. Write a function to print the output as a table with 10 rows and 5 columns. Column names are Total, Salary, Rate%, Raise, New Salary.
6. you may have more than the 3 functions mentioned in item 3, 4, 5.
7. DO NOT USE LOOPS.

Here's what I wrote:

Code:

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

//function prototypes
void inputsalary(float *, float *, float *, float *, float *, float *, float *);//user input

//print
void print(float s1, float s2, float s3, float s4, float s5, float s6, float s7, 
    float st, 
    float r1, float r2, float r3, float r4, float r5, float r6, float r7,
    float rs1, float rs2, float rs3, float rs4, float rs5, float rs6, float rs7,
    float rst,
    float ns1, float ns2, float ns3, float ns4, float ns5, float ns6, float ns7,
    float nst);
    
//return total of all seven salaries
float total7(float s1, float s2, float s3, float s4, float s5, float s6, float s7);

//calculate rates for each salary
void rate7(float s1, float s2, float s3, float s4, float s5, float s6, float s7,
    float *, float *, float *, float *, float *, float *, float *);

//calculate raise for each salary
void raise7(float s1, float s2, float s3, float s4, float s5, float s6, float s7,
    float r1, float r2, float r3, float r4, float r5, float r6, float r7,
    float *, float *, float *, float *, float *, float *, float *, float *);

void newsalary7(float s1, float s2, float s3, float s4, float s5, float s6, float s7,
    float rs1, float rs2, float rs3, float rs4, float rs5, float rs6, float rs7,
    float *, float *, float *, float *, float *, float *, float *,
    float *);

int main()
{
    //variables
    float s1, s2, s3, s4, s5, s6, s7; //Salaries 1 - 7
    float st; //Salary total

    float r1=0, r2=0, r3=0, r4=0, r5=0, r6=0, r7=0; //Rate% 1-7
    float rs1=0, rs2=0, rs3=0, rs4=0, rs5=0, rs6=0, rs7=0; //Raise 1-7
    float rst=0; //Raise total
    float ns1=0, ns2=0, ns3=0, ns4=0, ns5=0, ns6=0, ns7=0; //New Salary 1-7
    float nst=0; //New Salary total

    //functions 
    inputsalary(&s1, &s2, &s3, &s4, &s5, &s6, &s7); //User input
    
    st = total7(s1, s2, s3, s4, s5, s6, s7); //Calculate Salary total
    
    rate7(s1, s2, s3, s4, s5, s6, s7, &r1, &r2, &r3, &r4, &r5, &r6, &r7); //Calculate rate for each salary 1-7
    
    raise7(s1, s2, s3, s4, s5, s6, s7, // salaries 1-7
        r1, r2, r3, r4, r5, r6, r7, //rates 1-7
        &rs1, &rs2, &rs3, &rs4, &rs5, &rs6, &rs7, &rst ); //raises 1-7

    newsalary7(s1, s2, s3, s4, s5, s6, s7, // salaries 1-7
        rs1, rs2, rs3, rs4, rs5, rs6, rs7, // raises 1-7
        &ns1, &ns2, &ns3, &ns4, &ns5, &ns6, &ns7, //new salaries 1-7
        &nst); // New Salaries total
        
    
    print(s1, s2, s3, s4, s5, s6, s7, //salaries 1-7
        st, //salaries total
        r1, r2, r3, r4, r5, r6, r7, //rate% 1-7
        rs1, rs2, rs3, rs4, rs5, rs6, rs7, //Raise 1-7
        rst, //Raise total
        ns1, ns2, ns3, ns4, ns5, ns6, ns7, //New Salary 1-7
        nst //New Salary total
        ); //Print output
    
    system("PAUSE");
    return 0;
}

//user input
void inputsalary(float *s1, float *s2, float *s3, float *s4, float *s5, float *s6, float *s7)
{
    printf("Enter salary 1:");
    scanf("%f", &(*s1));
    printf("Enter salary 2:");
    scanf("%f", &(*s2));
    printf("Enter salary 3:");
    scanf("%f", &(*s3));
    printf("Enter salary 4:");
    scanf("%f", &(*s4));
    printf("Enter salary 5:");
    scanf("%f", &(*s5));
    printf("Enter salary 6:");
    scanf("%f", &(*s6));
    printf("Enter salary 7:");
    scanf("%f", &(*s7));
}

//return salary total 
float total7(float s1, float s2, float s3, float s4, float s5, float s6, float s7)
{
    return s1 + s2 + s3 + s4 + s5 + s6 + s7; 
}

//calculate rate for each salary
void rate7(float s1, float s2, float s3, float s4, float s5, float s6, float s7, 
    float *r1, float *r2, float *r3, float *r4, float *r5, float *r6, float *r7)
{
    //------------------------------- salary rate 1
    if(s1 < 30000)
    {
        *r1 = 7.0;
    }
    if(s1 >= 30000 && s1 <= 40000)
    {
        *r1 = 5.5;
    }
    if(s1 > 40000)
    {
        *r1 = 4.0;
    }
    //------------------------------- salary rate 2
    if(s2 < 30000)
    {
        *r2 = 7.0;
    }
    if(s2 >= 30000 && s2 <= 40000)
    {
        *r2 = 5.5;
    }
    if(s2 > 40000)
    {
        *r2 = 4.0;
    }
    //------------------------------- salary rate 3
    if(s3 < 30000)
    {
        *r3 = 7.0;
    }
    if(s3 >= 30000 && s3 <= 40000)
    {
        *r3 = 5.5;
    }
    if(s3 > 40000)
    {
        *r3 = 4.0;
    }
    //------------------------------- salary rate 4
    if(s4 < 30000)
    {
        *r4 = 7.0;
    }
    if(s4 >= 30000 && s4 <= 40000)
    {
        *r4 = 5.5;
    }
    if(s4 > 40000)
    {
        *r4 = 4.0;
    }
    //------------------------------- salary rate 5
    if(s5 < 30000)
    {
        *r5 = 7.0;
    }
    if(s5 >= 30000 && s5 <= 40000)
    {
        *r5 = 5.5;
    }
    if(s5 > 40000)
    {
        *r5 = 4.0;
    }
    //------------------------------- salary rate 6
    if(s6 < 30000)
    {
        *r6 = 7.0;
    }
    if(s6 >= 30000 && s6 <= 40000)
    {
        *r6 = 5.5;
    }
    if(s6 > 40000)
    {
        *r6 = 4.0;
    }
    //------------------------------- salary rate 7
    if(s7 < 30000)
    {
        *r7 = 7.0;
    }
    if(s7 >= 30000 && s7 <= 40000)
    {
        *r7 = 5.5;
    }
    if(s7 > 40000)
    {
        *r7 = 4.0;
    }
}



//calculate raise for each salary
void raise7(float s1, float s2, float s3, float s4, float s5, float s6, float s7,
    float r1, float r2, float r3, float r4, float r5, float r6, float r7,
    float *rs1, float *rs2, float *rs3, float *rs4, float *rs5, float *rs6, float *rs7,
    float *rst)
{
    *rs1 = s1*r1/(float)100;
    *rs2 = s2*r2/(float)100;
    *rs3 = s3*r3/(float)100;
    *rs4 = s4*r4/(float)100;
    *rs5 = s5*r5/(float)100;
    *rs6 = s6*r6/(float)100;
    *rs7 = s7*r7/(float)100;
    *rst = *rs1 + *rs2 + *rs3 + *rs4 + *rs5+ * rs6 + * rs7;
}


void newsalary7(float s1, float s2, float s3, float s4, float s5, float s6, float s7,
    float rs1, float rs2, float rs3, float rs4, float rs5, float rs6, float rs7, 
    float *ns1, float *ns2, float *ns3, float *ns4, float *ns5, float *ns6, float *ns7,
    float *nst)
{
    *ns1 = s1+rs1;
    *ns2 = s2+rs2;
    *ns3 = s3+rs3;
    *ns4 = s4+rs4;
    *ns5 = s5+rs5;
    *ns6 = s6+rs6;
    *ns7 = s7+rs7;
    *nst = *ns1 + *ns2 + *ns3 + *ns4 + *ns5 + *ns6 + *ns7;
}

//print output 
void print(float s1, float s2, float s3, float s4, float s5, float s6, float s7, 
    float st, 
    float r1, float r2, float r3, float r4, float r5, float r6, float r7,
    float rs1, float rs2, float rs3, float rs4, float rs5, float rs6, float rs7,
    float rst,
    float ns1, float ns2, float ns3, float ns4, float ns5, float ns6, float ns7,
    float nst)
{

    printf("                        Calculation of Salary Rasies\n\n");
    
    printf("           Salary       Rate%%       Raise         New Salary         \n");
    printf("                                                                      \n");
    printf("           %0.2f        %0.2f        %0.2f         %0.2f              \n", s1, r1, rs1, ns1);
    printf("           %0.2f        %0.2f        %0.2f         %0.2f              \n", s2, r2, rs2, ns2);
    printf("           %0.2f        %0.2f        %0.2f         %0.2f              \n", s3, r3, rs3, ns3);
    printf("           %0.2f        %0.2f        %0.2f         %0.2f              \n", s4, r4, rs4, ns4);
    printf("           %0.2f        %0.2f        %0.2f         %0.2f              \n", s5, r5, rs5, ns5);
    printf("           %0.2f        %0.2f        %0.2f         %0.2f              \n", s6, r6, rs6, ns6);
    printf("           %0.2f        %0.2f        %0.2f         %0.2f              \n", s7, r7, rs7, ns7);
    printf(" Total     %0.2f                     %0.2f         %0.2f              \n", st, rst, nst);

}

**laserlight** · 03-30-2012

Have you been taught about arrays? At a glance, it looks like the use of arrays will cut down quite a bit of duplicate code.

**iMalc** · 03-30-2012

Welcome, and thanks for your honest and well written opening statement. Believe it or not many just come here and rudly post their assignment description and nothing else. Others try and disguise that it is homework, which the people here are amazangly good at seeing right through.

Wow, that number 7 is a bummer (no loops). When instructors post such things I always really hope that the ONLY reason is that they want you to see how bad it is like that and how much better it will be later when loops are used instead. Even then I think it's a dodgy way to go about teaching it. I would never ask a student to avoid loops. I would sooner instruct them to use loops for an assignment that doesn't necessarily require them.
It doesn't seem to say no arrays though. Even without loops, arrays would still reduce the amount of code somewhat, particularly in the number of function parameters. You could also get sneaky and use recursion instead too

You should also make use of the 'else' keyword. Once you know that a variable was not less than 30000, then an else statement would avoid the need to test for the exact same thing again.

**smokeyangel** · 03-30-2012

Indeed, the "no loops" restriction is irritating. You can indeed put the data in arrays, which as said would reduce the number of parameters passed and eliminate a bit of code. I wouldn't personally throw myself down the recursion rabbit hole at this point.

Given the "no loops" restriction, your code is pretty much what I'd expect. It's great that you want to make it cleaner and more elegant, but I'd probably save my energy for when I'm allowed to use loops. It'll all make a lot more sense then!

**gordonfreeman** · 03-31-2012

Yeah No array's are allowed and neither is recursion. It certainly is ugly code. Thanks for the input :-)

**grumpy** · 03-31-2012

At a glance, you have a lot of duplicated code. You could eliminate some of that by using helper functions, even without loops.

For example, rate7() could be simplified by

Code:

float rate7_helper(float value)
{
    float retval;
    if(value < 30000f)
        retval = 7f;
    else if (value > 40000f)
        retval = 4.0f;
    else
         retval = 5.5f;  
}

void rate7(float s1, float s2, float s3, float s4, float s5, float s6, float s7,
    float *r1, float *r2, float *r3, float *r4, float *r5, float *r6, float *r7)
{
    *r1 = rate7_helper(s1);
    *r2 = rate7_helper(s2);
    *r3 = rate7_helper(s3);
    *r4 = rate7_helper(s4);
    *r5 = rate7_helper(s5);
    *r6 = rate7_helper(s6);
    *r7 = rate7_helper(s7);
}

Note that I've simplified the logic inside rate7_helper() as per iMalc's suggestion, and also made all float literals have an f (e.g. 2.0f).

In input salary, scanf("%f", &(*s1)); is equivalent to scanf("%f", s1); No need to dereference the pointer and take the address again. & and * operations are the inverse of each other (in this context, anyway).

You could simplify things further by using structs to group some variables. Passing structs (or pointers to structs) around can allow you to reduce the number of arguments your functions have. Using rate7() as an example, you could re-implement that to accept two arguments (one a struct, one a pointer to a struct). I'll leave that as an exercise. Even better, rate7() could be implemented as accepting a single struct and returning a struct.

I suggest using more descriptive names for your variables, function names, and function arguments. That would make the code a bit more self-documenting (i.e. easier for someone reading the code to understand what it is actually doing).

Don't use system("pause"). It only works under windows.

There's another possibility that crossed my mind to simplify the code further. But that one would be encouraging you to develop bad habits, so I'll refrain.

**gordonfreeman** · 03-31-2012

Thanks for the advice! I'll take another look at this code. This is just what I've been looking for.

...

Originally Posted by grumpy

Don't use system("pause"). It only works under windows.

What should I use instead of system("PAUSE")? Should I use something like getch(); instead?

Thanks again

**rags_to_riches** · 03-31-2012

What the hell is the point of the "no loops" requirement here?

If you've studied structs, you could make a struct of this data

Code:

float s1, s2, s3, s4, s5, s6, s7; //Salaries 1 - 7
float r1=0, r2=0, r3=0, r4=0, r5=0, r6=0, r7=0; //Rate% 1-7
float rs1=0, rs2=0, rs3=0, rs4=0, rs5=0, rs6=0, rs7=0; //Raise 1-7
float ns1=0, ns2=0, ns3=0, ns4=0, ns5=0, ns6=0, ns7=0; //New Salary 1-7

Would make it a little less unwieldy.

**oogabooga** · 03-31-2012

The fact that you have to add a comment to your variables to say what they actually are shows that the names are bad. I realize you wanted them short to make the function calls easier, but using arrays will achieve that even with descriptive names. Your function names are also bad.

Another oddity of your program is that it uses the float type instead of the double type. double is more usual unless there's a specific reason not to use it. If you do change to double, make sure you change your scanf format to "%lf" (that's an ell, not a one). The printf format stays the same, still just "%f".

So switching to double, using arrays and giving things reasonable names gives you a main something like this:

Code:

#define SIZE 7
int main(void) {
    double salaries[SIZE];
    double totalSalaries;
    double rates[SIZE];
    double raises[SIZE];
    double totalRaises;
    double newSalaries[SIZE];
    double totalNewSalaries;

    InputSalaries(salaries);
    totalSalaries = CalculateTotalSalaries(salaries);
    CalculateRates(salaries, rates);
    totalRaises = CalculateRaises(salaries, rates, raises);
    totalNewSalaries = CalculateNewSalaries(salaries, raises, newSalaries);
    PrintReport(salaries, totalSalaries,
                rates, raises, totalRaises,
                newSalaries, totalNewSalaries);
    return 0;
}

Which, IMO, is much more "eloquent".

And with arrays, (and using grumpy's idea but changing the name and types of the helper function), you could also use this trick:

Code:

double CalculateRate(double value) {
    if (value < 30000)
        return 7.0;
    else if (value > 40000)
        return 4.0;
    else
        return 5.5;  
}

void CalculateRates(double *salaries, double *rates) {
    rates++ = CalculateRate(salaries++);
    rates++ = CalculateRate(salaries++);
    rates++ = CalculateRate(salaries++);
    rates++ = CalculateRate(salaries++);
    rates++ = CalculateRate(salaries++);
    rates++ = CalculateRate(salaries++);
    rates   = CalculateRate(salaries);
}

**gordonfreeman** · 03-31-2012

yeah I'm not allowed to use arrays yet but i still think that you've provided me with valuable advice. Thanks a lot :-D

Thread: How can I make this simple assignment more eloquent ?

Thread Tools

Search Thread

Display

How can I make this simple assignment more eloquent ?

Similar Threads

Simple C assignment, need help!

Simple help with an assignment

can any1 plz make this assignment

using swap to make assignment operator exception safe

This one simple thing I cant do for my assignment need help

Tags for this Thread