2

I tried implementing fizzbuzz in C++ and am confused by the different outputs the following code samples produce:

int main()
{
    int val1 = 1;
    while (val1 < 101) {
        if (val1 % 15 == 0)
            cout << "FizzBuzz\n";
        if (val1 % 3 == 0)
            cout << "Fizz\n";
        if (val1 % 5 == 0)
            cout << "Buzz\n";
        else cout << val1 << "\n";
        ++val1;
    }
    keep_window_open();
}

For all multiples of 15, this code outputs

FizzBuzz
Fizz
Buzz

instead of just FizzBuzz. All multiples of (only) 5 are correctly replaced with Buzz but all multiples of 3 print out Fizz and then the number itself.

The following code, however, works perfectly. I get that the else if makes it work correctly, but I'm just not seeing what the code path is when val1 = 3, or 5, or 15.

int main()
{
    int val1 = 1;
    while (val1 < 101) {
        if (val1 % 15 == 0)
            cout << "FizzBuzz\n";
        else if (val1 % 3 == 0)
            cout << "Fizz\n";
        else if (val1 % 5 == 0)
            cout << "Buzz\n";
        else cout << val1 << "\n";
        ++val1;
    }
    keep_window_open();
}
cdlane
  • 40,441
  • 5
  • 32
  • 81
kevin
  • 147
  • 2
  • 11
  • In the first case all lines are executed except for the `cout << val1 << "\n"`. The syntax is `if (condition) block` with optional `else other_block` . In your first case the first two `if` statements have no "else" so execution just goes onto the next statement. – M.M Aug 11 '15 at 04:33

4 Answers4

3

In the first example:

    if (val1 % 15 == 0)
        cout << "FizzBuzz\n";
    if (val1 % 3 == 0)
        cout << "Fizz\n";
    if (val1 % 5 == 0)
        cout << "Buzz\n";
    else cout << val1 << "\n";

every cases will be tested each and every iteration. Since all multiples of 15 are also multiples of 3 and 5 you'll have all three printed on the screen.

In the other case:

    if (val1 % 15 == 0)
        cout << "FizzBuzz\n";
    else if (val1 % 3 == 0)
        cout << "Fizz\n";
    else if (val1 % 5 == 0)
        cout << "Buzz\n";
    else cout << val1 << "\n";

the first case (val1 % 15 == 0) will be tested, and if it's false the second one will be tested, and if it's false the third one will be tested. Which means that only the first from top to bottom of the four cases will ever test true per iteration.

For example, if you were to reverse the order of 5 and 15:

    if (val1 % 5 == 0)
        cout << "FizzBuzz\n";
    else if (val1 % 3 == 0)
        cout << "Fizz\n";
    else if (val1 % 15 == 0)
        cout << "Buzz\n";
    else cout << val1 << "\n";

you would get that Buzz would never be printed, because if the value is divisible per 15 you would stop a the very first val1 % 5 == 0 test, therefore printing FizzBuzz.

Shoe
  • 74,840
  • 36
  • 166
  • 272
1

The code first catches numbers which are multiples of 15, which means they are multiples of both 3 and 5. In this case, it prints "FizzBuzz". Any number which fails this test may be either a multiple of 3 or 5 but not both. The next two if statements check for multiples of 3 or 5, respectively

while (val1 < 101) {
    if (val1 % 15 == 0)           // multiple of 3 AND 5
        cout << "FizzBuzz\n";
    else if (val1 % 3 == 0)       // multiple of ONLY 3
        cout << "Fizz\n";
    else if (val1 % 5 == 0)       // multiple of ONLY 5
        cout << "Buzz\n";
    else cout << val1 << "\n";
    ++val1;
}
Tim Biegeleisen
  • 502,043
  • 27
  • 286
  • 360
1

Your else belongs only to the last if:

if (val1 % 15 == 0)
    cout << "FizzBuzz\n";

if (val1 % 3 == 0)
    cout << "Fizz\n";

if (val1 % 5 == 0)
    cout << "Buzz\n";
else // any val1 which doesn't divide by 5
    cout << val1 << "\n";

That's why, if val1 is not divisible by 5, it outputs this value.

Also, 15 is divisible either by 15, 5 and 3. It triggers all three ifs and outputs all three lines.

You can use else if in order to get the correct results but the best way is to replace it with terminating execution on completed condition. For example, if you had a function you would be able to do it this way:

string GetOutput(int val)
{
    if (val % 15 == 0) return "FizzBuzz";
    if (val % 3 == 0) return "Fizz"; // 2. val % 15 != 0 implied
    if (val % 5 == 0) return "Buzz"; // 3. val % 15 != 0 && val % 3 != 0 implied
    return to_string(val1); // 4. val % 15 != 0 && val % 5 != 0 && val % 3 != 0 implied
}

int main() {
   cout << GetOutput(val) << endl;
}

It would work because execution terminates on true condition, and any condition check implies that all previous are false. These two rules are guaranteed to be correct in this example:

1. If execution is at lines 2 or 3 - val is not divisible by 15
2. If execution is at line 4 - val is not divisible by 3, 5 and 15

With this approach, you don't need to describe these conditions manually.
Moreover, if you have more and more conditions, it will be much easier to maintain and read such function than write very long logic condition.

For example,

string GetOutput(int val)
{
    if (val % 64 == 0) return "FizzBuzz"; // such wow
    if (val % 32 == 0) return "Fizz"; // much readable
    if (val % 16 == 0) return "Buzz";
    if (val % 8 == 0) return "Muzz";
    if (val % 4 == 0) return "Guzz";
    if (val % 2 == 0) return "Duzz";
    return "Hizz";
}

instead of

if (val % 64 == 0) cout << "FizzBuzz";
if (val % 64 != 0 && val % 32 == 0) cout << "Fizz";
if (val % 64 != 0 && val % 32 != 0 && val % 16 == 0) cout << "Buzz";
if (val % 64 != 0 && val % 32 != 0 && val % 16 != 0 && val % 8 == 0) cout << "Muzz";
if (val % 64 != 0 && val % 32 != 0 && val % 16 != 0 && val % 8 != 0 && val % 4 == 0) cout << "Guzz";
if (val % 64 != 0 && val % 32 != 0 && val % 16 != 0 && val % 8 != 0 && val % 4 != 0 && val % 2 == 0) cout << "Duzz";
if (val % 64 != 0 && val % 32 != 0 && val % 16 != 0 && val % 8 != 0 && val % 4 != 0 && val % 2 != 0) cout << "Hizz";

or

if (val % 64 == 0) 
{
    cout << "FizzBuzz";
} else {
    if (val % 32 == 0)
    {
        cout << "Fizz";
    } else {
        if (val % 16 == 0) 
        {  
            cout << "Buzz";
        } else {
            if (val % 8 == 0)
            {
                cout << "Muzz";
            } else {
                if (val % 4 == 0)
                {
                    cout << "Guzz";
                } else { 
                    if (val % 2 == 0)
                    {
                        cout << "Duzz";
                    } else {
                        cout << "Hizz";
                    }
                }
            }
        }
    }
}
Yeldar Kurmangaliyev
  • 33,467
  • 12
  • 59
  • 101
1

A multiple of 15 is also a multiple of 5 and a multiple of 3. The else says "only when the previous if wasn't true then do.."; thus removing it in a series of non-mutually exclusive conditionals changes the logic.

However, consider that the conditions could be made mutually exclusive:

if (val1 % 15 == 0)
    cout << "FizzBuzz\n";

if (val1 % 3 == 0 && !(val1 % 15 == 0))
    cout << "Fizz\n";

if (val1 % 5 == 0 !(val1 % 15 == 0))
    cout << "Buzz\n";

if (!(val1 % 3 == 0) && !(val1 % 5 == 0)) // implies !(val1 % 15 == 0)
    cout << val1 << "\n";

In this case, for any integer value, only one of the if conditions will be true.

I also changed the final else to show the mutual exclusion extended across all paths. That being said, I would not recommend writing code like this.


From another viewpoint, every else if sequence can be written as a 'nested tree'. The semantics are exactly identical, but the structure may make the path easier to visualize. In this case it should be clear that every terminal case (a "cout") is mutually exclusive from every other.

if (val1 % 15 == 0) {
    cout << "FizzBuzz\n";
} else {
    if (val1 % 3 == 0) {
        cout << "Fizz\n";
    } else {
        if (val1 % 5 == 0) {
            cout << "Buzz\n";
        } else {
            cout << val1 << "\n";
        }
    }
}
user2864740
  • 60,010
  • 15
  • 145
  • 220