Unexpected Outputs in a simple FizzBuzz program in C++

70 views Asked by At

I was just trying my hand at implementing a simple FizzBuzz program where I'm trying to separate the ruleset from the logic and here is my attempt at doing this within Compiler Explorer, no use of any IDE or debugging tools.

#include <cstdint>
#include <map>
#include <string>

typedef std::map<uint32_t,std::string> RuleSet;

RuleSet ruleSet { {3, "fizz"},
                  {5, "buzz"}
};

void fizzBuzz(int32_t max_limit, RuleSet rules) {
    std::string output{""};
    for (uint32_t i = 1; i < max_limit; i++) {        

        for (auto r : rules) {
            if ( (i % r.first) == 0 ) {
                output = r.second;
            } else {
                output = std::to_string(i);
            }
        }
        output.append(1, '\n');
        std::cout << output;
        output.clear();
    }
}

int main() {
    fizzBuzz(100, ruleSet);
    return 0;
}

Here's a link to it in compiler explorer: compiler explorer

Within the execution unit, It's printing the number values, and it's printing buzz, but for the life of me, I'm trying to figure out why it's not printing fizz anywhere... Any kind of hint or suggestion or insight as to why I'm getting the output that am I would be helpful. Please take note that I'm not writing this or running it in an IDE as I currently do not have access to a debugger to step through my code. This was implemented directly in Compiler Explorer. I know I'm overlooking something but any help at this point is appreciated.


The expected output should be:

1
2
fizz
4
buzz
fizz
7
8
fizz
buzz
11
fizz
13
14
fizzbuzz
// and so on...

Here's the actual output that I'm getting from Compiler Explorer:

1
2
3
4
buzz
6
7
8
9
buzz
11
12
13
14
buzz
16
17
18
19
buzz
21
22
23
24
buzz
26
27
28
29
buzz
31
32
33
34
buzz
36
37
38
39
buzz
41
42
43
44
buzz
46
47
48
49
buzz
51
52
53
54
buzz
56
57
58
59
buzz
61
62
63
64
buzz
66
67
68
69
buzz
71
72
73
74
buzz
76
77
78
79
buzz
81
82
83
84
buzz
86
87
88
89
buzz
91
92
93
94
buzz
96
97
98
99
2

There are 2 answers

5
user12002570 On

The problem is that you're not breaking out of the inner for loop when the condition (i % r.first) is met. This can be easily seen either using a debugger or adding std::cout.

Basically you need to break out of the inner for loop as soon as the condition is satisfied(after setting the output in your example).

Thus to get your expected output, just add a break statement inside the if block as shown below:

for (auto r : rules) {
    if ( (i % r.first) == 0 ) {
        output = r.second;
//------vvvvv------------------>added this break statement 
        break;
    } else {
        output = std::to_string(i);
    }
}

Working demo

After making this change, the output is as in your expected output:

1
2
fizz
4
buzz
fizz
7
8
fizz
buzz
11
fizz
13
14
fizz
16
17
fizz
19
buzz
fizz
22
23
fizz
buzz
26
fizz
28
29
fizz
31
32
fizz
34
buzz
fizz
37
38
fizz
buzz
41
fizz
43
44
fizz
46
47
fizz
49
buzz
fizz
52
53
fizz
buzz
56
fizz
58
59
fizz
61
62
fizz
64
buzz
fizz
67
68
fizz
buzz
71
fizz
73
74
fizz
76
77
fizz
79
buzz
fizz
82
83
fizz
buzz
86
fizz
88
89
fizz
91
92
fizz
94
buzz
fizz
97
98
fizz
0
James Kanze On

The problem is that you're overwriting the results every time you go through the inner loop, so only the last rule has any effect. What you probably want to do is use append, and then convert from the number if the results are still empty. Something like the following:

for ( int i = 1; i < max_limit; ++ i ) {
    std::string         output;
    for ( auto r: rules ) {
        if ( i % r.first == 0 ) {
            output += r.second;
        }
    }
    if ( ! output.empty() ) {
        std::cout << output << std::endl;
    } else {
        std::cout << i << std::endl;
    }
}

I might add that you probably want to pass Rules by const reference, rather than by value.