Is there a better way to write these If-Statements?

Question

I'm building a Random Character Generator in C++, and I have around 12 large blocks of if statements, like this:

int wisdom = rand() % 18;

cout << "\n";

if (wisdom == 0 || wisdom == 1) {

    cout << "Wisdom Score: 1\n";
    cout << "Modifier: -5\n";

} else if (wisdom == 2 || wisdom == 3) {

    cout << "Wisdom Score: " << wisdom << "\n";
    cout << "Modifier: -4\n";

} else if (wisdom == 4 || wisdom == 5) {

    cout << "Wisdom Score: " << wisdom << "\n";
    cout << "Modifier: -3\n";

} else if (wisdom == 6 || wisdom == 7) {

    cout << "Wisdom Score: " << wisdom << "\n";
    cout << "Modifier: -2\n";

} else if (wisdom == 8 || wisdom == 9) {

    cout << "Wisdom Score: " << wisdom << "\n";
    cout << "Modifier: -1\n";

} else if (wisdom == 10 || wisdom == 11) {

    cout << "Wisdom Score: " << wisdom << "\n";
    cout << "Modifier: +0\n";

} else if (wisdom == 12 || wisdom == 13) {

    cout << "Wisdom Score: " << wisdom << "\n";
    cout << "Modifier: +1\n";

} else if (wisdom == 14 || wisdom == 15) {

    cout << "Wisdom Score: " << wisdom << "\n";
    cout << "Modifier: +2\n";

} else if (wisdom == 16 || wisdom == 17) {

    cout << "Wisdom Score: " << wisdom << "\n";
    cout << "Modifier: +3\n";

} else {

    cout << "Wisdom Score: 18\n";
    cout << "Modifier: +4\n"; 

}

I'm wondering, is there a better way to write this? Perhaps some type of function?

This would be a good question to ask on [codereview.se], since you already have working code that you'd like to improve. It's specifically for these open-ended "how should I improve this code" sorts of questions. — Sneftel, Jun 28 '22 at 17:23
I’m voting to close this question because it is working code asking for a [codereview.se] — Daniel A. White, Jun 28 '22 at 17:37
Consider that when you mention *character* to a c++ developer in a generic context, he'll need a couple of seconds to realize that you meant a mage in a wood instead of a `char` — MatG, Jun 28 '22 at 18:07
@MatG Oops. Yep, I see where that could be confusing, my bad. — HoneycodeRidge, Jun 28 '22 at 18:11

ShadowRanger · Accepted Answer · 2022-06-28T18:38:38.270

10

The better way is to not write chained ifs at all, but instead compute the value you care about.

if (wisdom == 0) wisdom = 1; // Handle edge case treating 0 as 1
int modifier = wisdom / 2 - 5;

cout << "Wisdom Score: " << wisdom << '\n';
cout << "Modifier: " << modifier << '\n';

Note that your calculation of int wisdom = rand() % 18; cannot produce 18 (and does produce 0, which you don't want), so you probably want to change it to:

int wisdom = rand() % 18 + 1;  // Result guaranteed to be 1-18 inclusive

allowing you to simplify the code by removing the if (wisdom == 0) wisdom = 1; edge case.

As another answer has already noted, rand is typically considered a bad API, so unless you're okay with biased results (e.g. slightly more low rolls than high rolls), you'll want to use modern C++ PRNG APIs (they're a little more work to set up, but trivial to use once you've done so, and they should avoid bias issues).

edited Jun 28 '22 at 18:38

answered Jun 28 '22 at 17:25

ShadowRanger

143,180
12
188
271

`(wisdom - 10) / 2` gives incorrect results for `wisdom < 10`. `-1 / 2` is `0` but the wisdom modifier for `9` is supposed to be `-1`. – Nathan Pierson Jun 28 '22 at 17:26
@NathanPierson: Ah, yeah, got used to Python (which guarantees floor division). Fixed. – ShadowRanger Jun 28 '22 at 17:27
@MarkTolonen: The edge case handling is needed only because the original code would output `Wisdom: 1` when `wisdom` was `0`. In all likelihood, the code should really just produce the 1-18 value directly and exclude the possibility of `0`. Granted, I handled it "wrong" by leaving `wisdom` itself as `1`, not merely reporting it as such; to leave it unchanged, you could do: `cout << "Wisdom Score: " << (wisdom ? wisdom : 1) << '\n';`, but since the OP clearly wants `18` to be a possibility and doesn't *really* seem to want `0`, neither is needed if `wisdom` is computed correctly the first time. – ShadowRanger Jun 28 '22 at 17:30
ah, didn't see that it printed differently. – Mark Tolonen Jun 28 '22 at 17:33

score 5 · Answer 2 · answered Jun 28 '22 at 17:24

5

Use the fact that the division of 2 integral values results in the trucation of the result:

...
int modifier = (wisdom / 2) - 5;

answered Jun 28 '22 at 17:24

fabian

80,457
12
86
114

I'd also note that this conversion from wisdom to wisdom modifier is a really good candidate to make into a function because I have a hunch that OP will want to repeat this code five more times with charisma, dexterity, etc. – Nathan Pierson Jun 28 '22 at 17:35

bitmask · Answer 3 · 2022-06-28T17:38:00.987

3

You don't need any conditionals. Also, don't use rand() %.

std::mt19937 mt(42); // seed
auto const wisdom = std::uniform_int_distribution<int>(0,18)(mt);
auto const score = wisdom + !wisdom;
auto const mod = wisdom / 2 - 5;

std::cout << "Wisdom Score: " << score << "\n";
std::cout << "Modifier: " << mod << "\n";

This assumes it is intentional that you have a double chance of wisdom score 1 proccing compared to other scores. If not change the lower bound in the std::uniform_int_distribution construction from 1 to 0.

edited Jun 28 '22 at 17:38

answered Jun 28 '22 at 17:28

bitmask

32,434
14
99
159

What is the `wisdom += !wisdom;` doing here, and is it necessary? – Nathan Pierson Jun 28 '22 at 17:29
@NathanPierson he's using it to ensure > 0 (in admittedly a convoluted way). – WhozCraig Jun 28 '22 at 17:30
OP wants a wisdom of `0` to always map to `1`. Adding `!wisdom` is the easiest branch-free way that came to mind. There are possibly others. – bitmask Jun 28 '22 at 17:30
1

@RemyLebeau OP clearly wants 1 to have double chance of proccing. So, no, (1,18) would not reflect OP's code. – bitmask Jun 28 '22 at 17:31
@MarkTolonen yes, for the modifier, but the OP also wants `0` to have a score of `1` not `0` – Remy Lebeau Jun 28 '22 at 17:32
I would not say that it's "clear" the OP wants that. You presumably do not think it's "clear" that the OP wants a 0% chance of rolling an 18. – Nathan Pierson Jun 28 '22 at 17:32
@NathanPierson Fair point with the 18. I assumed it was an oversight given they added an extra branch for `18`. Let's say, I think OP wants 1 to have double chance. Otherwise change the lower bound to 1 as Remy suggested. – bitmask Jun 28 '22 at 17:34
Thank you everyone, I'm new to this all so this is my first larger program writing from scratch. I appreciate everyone's input! – HoneycodeRidge Jun 28 '22 at 17:47

Is there a better way to write these If-Statements?

3 Answers3