16

I'm trying to make a function that determines commission based on user entered amount. It takes a user entered double and uses it to determine which equation it is used in. But the code I wrote always goes to the else statement, and I am not sure what is wrong with my conditions.

double calculate(double s)
{
    double c;
    if (s > 300,000)
    {
        c = 25,000 + (0.15 * (s-300,000));
        cout << "went to if" << endl;
        return c;

    }

    else if (300,000 > s && s > 100,000)
    {
        c = 5,000 + (0.10 * (s-100,000));
        cout << "went to else if" << endl;
        return c;

    }

    else
    {
        c = 0.05 * s;
        cout << "went to else" << endl;
        return c;

    }
} 
10
  • @krzaq How?? Seriously? Oct 23, 2016 at 20:15
  • 3
    @πάνταῥεῖ both are about misuse of operator ,.
    – krzaq
    Oct 23, 2016 at 20:16
  • 1
    @krzaq " feels like a minefield" It is one of course :) Oct 23, 2016 at 20:20
  • 7
    @krzaq This question is just so naive and cute, it deserved an answer :P Oct 23, 2016 at 20:24
  • 3
    Ugh...it is so obvious in retrospect...
    – gingikid
    Oct 23, 2016 at 20:27

3 Answers 3

25

s > 300,000 is a comma expression, equivalent to (s > 300),000. The value of a comma expression is the value of the last one in the list, here 000. Which evaluates as false when converted to bool.

You can write that as

if( s > 300'000 )

or, if the compiler doesn't support that newfangled notation, as just

if( s > 300000 )

Or you can define

double const k = 1000;

and write

if( s > 300*k )

And similarly for the 25 000, 100 000 and 5000 literals.

3
  • That worked, thank you! I should have checked that when I was testing it. I ran the program the first time, entering the amount with a comma in it, and got an incorrect amount. I should have realized it then!
    – gingikid
    Oct 23, 2016 at 20:18
  • 1
    Also, you could declare the string literal "k" as a user-defined literal to multiply the given integer by 1000. Oct 23, 2016 at 20:39
  • @StackDanny: Yes, thanks, good idea, but I think the above is the easiest to understand when looking at the source code (as opposed to writing it). Maybe. Not sure. Oct 23, 2016 at 20:42
18

What was intended as a number 300,000 in

if (s > 300,000)

is, in fact, a curious use of ,-operator, which is parsed as

if ((s > 300),(000))

and results in false all the time. Instead, try

if (s > 300000)

(Ditto for else if (300,000 > s && s > 100,000) and in a couple of other places.)

4

300,000 isn't a proper double literal, but applies the comma operator and ends up with the value 0.

Use a . for double literals: 300.000 or no separator for thousands: 300000

3
  • 2
    It could be that a thousand-separator was meant, not ..
    – AlexD
    Oct 23, 2016 at 20:20
  • @AlexD I mentioned that? Oct 23, 2016 at 20:21
  • @AlexD Better, yes ;) Oct 23, 2016 at 20:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.