0

I am having some trouble with a basic program for a C++ class. In the following function, i have debugged and the arguments being passed in are correct, however the if statement below is returning 'F' for the first element of the array I pass in, and the second and each thereafter score I pass in is being returned as 'D'.

When I do the math out, my data set should hit each part of the if statement at least once, however im guessing maybe my PEMDAS is off?

Here is the if statement:

char grade(double score, double m, double s) {

    if (score < (m - 1.5 * s)) {
        return 'F';
    }
    if ((m - (1.5*s)) <= score < (m - (0.5 * s))) {
        return 'D';
    }
    if ((m - (0.5 * s)) <= score < (m + (0.5 * s))) {
        return 'C';
    }
    if ((m + (0.5 * s)) <= score < (m + (1.5 * s))) {
        return 'B';
    }
    if ((m + (1.5 * s)) <= score) {
        return 'A';
    }
    else {
        return 'X';
    }
}
melpomene
  • 84,125
  • 8
  • 85
  • 148
Logan Crone
  • 11
  • 3
    `[...] <= score < [...]` comparisons don't work that way in c++. – tkausl Oct 27 '17 at 00:50
  • 1
    `A < B < C` ( or any comparison operator, not just `<`) doesn't do what you think it does, in C++. [See this link for more information](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) – Sam Varshavchik Oct 27 '17 at 00:50
  • What would be my best bet for this situation? – Logan Crone Oct 27 '17 at 00:52
  • 1
    two seperate comparisons and a `&&` – tkausl Oct 27 '17 at 00:55
  • You're a god, I remember the && now but just figured I could compare as I did above. Thanks for the reminder! – Logan Crone Oct 27 '17 at 00:57
  • When you write `x < y < z`, it is actually calculated as `(x < y) < z`. If `x` is less than `y`, the expression `(x < y)` returns `true`, which is converted to `1` in arithmetic and comparison expressions. If `x` is no less than `y`, `(x < y)` returns `false`, which is converted to `0`. So your expression becomes, effectively, `1 < z` and `0 < z`, respectively. Use `(x < y) && (y < z)` instead. – Headcrab Oct 27 '17 at 00:58

3 Answers3

2

An expression like:

a < b < c

does not do what you think it does. It actually evaluates as:

(a < b) < c

where a < b is a truth value giving 0 for false and 1 for true`. Then that value is used in the rest of the expression.

By way of example, the expression 100 < 200 < 42 will be true because 100 < 200 evaluates to 1, which is definitely less than 42.

Instead you should be using something like:

(a < b) && (b < c)

In any case, you can clean up that code quite a bit by realising that:

  1. The construct if (condition) return else ... is totally unnecessary - the return means that the rest of the code will execute only if condition is false, rendering the else superfluous.

  2. Because your ranges are mutually exclusive, the checks can be simplified. In other words, no need to check if (m - (1.5*s)) <= score in the D case since, if that weren't true, the code would already have returned F.

  3. It is currently impossible to get an X grade since there is no score that doesn't get captured one of the if statements, excepting possibly some edge cases like NaN but I'm going to assume you're not worried about that.

With those points in mind, the code can be simplified to:

char grade(double score, double m, double s) {
    if (score < m - 1.5 * s) return 'F';
    if (score < m - 0.5 * s) return 'D';
    if (score < m + 0.5 * s) return 'C';
    if (score < m + 1.5 * s) return 'B';
    return 'A';
}
paxdiablo
  • 854,327
  • 234
  • 1,573
  • 1,953
0

We would need to know the input data values to have a complete understanding, however as others have pointed out, comparisons don't work like that in C++.

Take this comparison: (m - (1.5*s)) <= score < (m - (0.5 * s))

What this is doing is first evaluating: (m - (1.5*s)) <= score

Due to the first if statement failing, this will always evaluate to true. In C++, boolean values are integers, and true is usually represented by "1" (in practice it doesn't have to be, and can be represented by any non-zero number, but I think in this specific case it's being set to 1).

Then it's evaluating: 1 < (m - (0.5*s))

Which I'm assuming always returns true given your values of "m" and "s".

What you should be doing instead is using a compound statement:

if (m - 1.5*s <= score && score < m - 0.5*s)

(you can keep the parentheses if you like, but they're unnecessary here due to C++'s operator precedence rules)

Kimby
  • 124
  • 1
  • 4
0

Just drop all of the lower-bound comparisons (which, as others have said, do not do what you thought): if any of them don’t apply, you would already have returned one of the lower scores.

Davis Herring
  • 36,443
  • 4
  • 48
  • 76