You should put main
at the bottom. It's not useful to declare your functions in a CPP file, since C++ supports overloading. If you defined the actual function with a different signature that would not be a compile-time error. But you would have a linker error -- the pre-declaring of the function did not help you, and only caused more confusion.
So, put functions in the order they need to be. Declarations in the CPP files are only for mutual recursion.
So much of what's poor with the solution is due to the specification! I understand that a beginner can't swallow everything at once, and it's trying to teach functions and decomposition in this lesson.
Your function should have three arguments: the price as a reference to a double, ...
Never never never use double
for money.
I was shocked at how many pros in business did not learn this. So don't flirt with such an insidious habit even in toy code.
And making it a reference is suspicious.
...the discount as a double value, ...
That seems like it might be OK for a rate, but it runs into some of the same issues. E.g. you can't represent 10% exactly.
...and a bool to indicate if the discount is calculated as a percentage or a fixed amount.
You're using the same type to represent an amount or a discount. Are they the same type of thing? Even if they can be represented by the same underlying primitive type, they are not the same kind of thing. You're writing code using primitives in the declaration, not domain-specific types. You are not passing a double
but an Amount
, logically. A book that teaches problem solving should introduce that idea early, too.
As for using a bool
to indicate two modes of operation for the function: Well, that does happen. A bool
can be OK here, but only just. It's really poor design to have two different functions under one wrapper, without having a majority of the code in common.
BTW, I happen to have exactly that in my own code! But, it's not passing a amount-or-rate as a single parameter. The amount, or the rate, is found in a database. In my case, I use an enum
to indicate Rate
or Discount
(or whatever I called them). And the function in question called the underlying calculation as one line, and the rest of the function was doing work that didn't matter which it was, so it's sensible to combine them into one function like that.
You should calculate the discount and modify the price of the item accordingly.
Don't use "in-out" parameters! This is poor design. It should be returning the new value. You are returning double
too even though this spec does not say anything about a return value.
Your program should check that the discount is not negative, and that the price of the item does not drop to zero after applying discount. Use assert
statement for this.
Checking is good. The assert
statement, not so much. It only has an effect in a debug build and is ignored in a release build! It's difficult to unit test that your code performs that check, as it simply aborts the program. An exception would be better. There is an exception type designed for exactly this.
Are the requirements clear?
You write (price * (discount/100.0));
interpreting the discount
parameter as 30.0 meaning 30%. But it's a double
, and it's normal to use a floating-point value to mean a value between 0.0 and 1.0. That is, 30% would naturally be 0.3.
Jarod42 already pointed out that you are asking for the type
parameter inside the function. You are not passing it to the function! In main
you declare it as char
(not bool
) but don't use it.
You are missing the point of having a function. You should collect the needed information and hand it all to the function. The function should look only at its parameters and not worry about where the information came from.
Then you prompt for discount
! You are completely missing the point of using a function that takes discount
as a parameter!
Worse, you are not computing the discount amount in the non-percent case. You are neither computing the amount nor checking for a negative value as directed. Your code gives the wrong answer and fails.
You should not post on Code Review until code is complete and working correctly. You don't even know... this shows why you need testing. One big advantage of using functions (correctly) is you can call them from test code with built-in values which are checked automatically, not prompting you for anything at all. This lets the program auto-test the logic without you having to repeatedly type in values.
You write: return(discount);
But return
is not a function. Write return discount;
without redundant parens around the whole thing. This does make a difference in some cases as of C++11.
Don't Give Up
This is the way to learn these things. The books don't tell you everything, and trying things for yourself is the only way to figure it out.