Like Tree1Likes
  • 1 Post By oogabooga

How can I make this simple assignment more eloquent ?

This is a discussion on How can I make this simple assignment more eloquent ? within the C Programming forums, part of the General Programming Boards category; Hey guys I'm new to this forum and I'd like to find out if someone out there can make the ...

  1. #1
    Registered User
    Join Date
    Feb 2012
    Posts
    8

    How can I make this simple assignment more eloquent ?

    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);
    
    }

  2. #2
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,794
    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.
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  3. #3
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,307
    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.
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  4. #4
    Registered User
    Join Date
    Mar 2010
    Posts
    535
    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!

  5. #5
    Registered User
    Join Date
    Feb 2012
    Posts
    8
    Yeah No array's are allowed and neither is recursion. It certainly is ugly code. Thanks for the input :-)

  6. #6
    Registered User
    Join Date
    Jun 2005
    Posts
    6,337
    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.
    Last edited by grumpy; 03-31-2012 at 02:49 AM.
    Right 98% of the time, and don't care about the other 3%.

  7. #7
    Registered User
    Join Date
    Feb 2012
    Posts
    8

    Cool

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

    ...

    Quote Originally Posted by grumpy View Post

    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

  8. #8
    a_capitalist_story
    Join Date
    Dec 2007
    Posts
    2,651
    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.

  9. #9
    - - - - - - - - oogabooga's Avatar
    Join Date
    Jan 2008
    Posts
    2,808
    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);
    }
    stahta01 likes this.
    The cost of software maintenance increases with the square of the programmer's creativity. - Robert D. Bliss

  10. #10
    Registered User
    Join Date
    Feb 2012
    Posts
    8
    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

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Simple C assignment, need help!
    By dallonhamilton in forum C Programming
    Replies: 4
    Last Post: 12-06-2010, 11:45 AM
  2. Simple help with an assignment
    By Anaesthetic in forum C++ Programming
    Replies: 4
    Last Post: 09-13-2010, 08:04 PM
  3. can any1 plz make this assignment
    By jean in forum C Programming
    Replies: 17
    Last Post: 05-13-2009, 09:19 PM
  4. using swap to make assignment operator exception safe
    By George2 in forum C++ Programming
    Replies: 9
    Last Post: 01-10-2008, 05:32 AM
  5. This one simple thing I cant do for my assignment need help
    By Rumproast23 in forum C++ Programming
    Replies: 9
    Last Post: 09-27-2006, 09:41 PM

Tags for this Thread


1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21