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.

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.

Originally Posted by Bjarne Stroustrup (2000-10-14)

I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.

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"

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!

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 03:49 AM.

Right 98% of the time, and don't care about the other 3%.

If I seem grumpy or unhelpful in reply to you, or tell you you need to demonstrate more effort before you can expect help, it is likely you deserve it. Suck it up, Buttercup, and read this, this, and this before posting again.

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: