FizzBuzzWoofFooBar - follow-up

I took all your suggestions to heart and I think it improved the code a lot.

The Map got it's own typedef: IntStrDict. It's a perfectly applicable name, but I can't help but feel it's not according to best practices.

The code now accepts arguments. If no arguments are given, the code fails. If one or more argument is given it accepts the first argument as number and ignores the rest. If a non-number is inserted as first argument it should break. Recommended argument is 15016 (the result of 3571113).

I'm not sure my FizzDict in main() is according to best practices either. I'm still looking for a way to make it more generic. Perhaps OO-style with a getFizzDict would be appropriate. An argument which allows you to choose between dictionaries would be fancy, but could get very ugly very fast as well.

Another difference is I finally split up the code into a .cpp and a .h. Many functions are split up now.

Is my exception handling up to standards?

I'm trying to follow SRP as much as sensible.

FizzBuzz.cpp

#include "FizzBuzz.h"

int main(int argc, char* argv[])
{
    int iMax;
    try {
        iMax = atoi(argv[1]);
    }
    catch (int e) {
        std::cout << "Exception " << e << '\n';
    }
    if (argc < 2) {
        throw 0;
    }

    const static IntStrDict FizzDict = {
        { 3, "Fizz" },
        { 5, "Buzz" },
        { 7, "Woof" },
        { 11, "Foo" },
        { 13, "Bar" }
    };

    for (int i = 1; i < iMax; ++i)
    {
        printString(FizzBuzzer(i, FizzDict));
    }
}

FizzBuzz.h

```
#ifndef _fizzbuzz_h_
#define _fizzbuzz_h_

#include
#include
#include

typedef std::map IntStrDict;

void printString(std::string tString)
{
std::cout << tString << '\n';
}

std::string ifEmpty(int i, std::string tString)
{
if (tString.empty())
{
tString += std::to_string(i);
}
return tString;
}

std::string checkModZero(int i, IntStrDict tDict)
{
std::string tString;
for (auto const &j : tDi

Here are some things I see that could help you improve your code.

Provide some guidance for the user

The user of the program might not know that this is expecting a numeric argument. Instead of throwing an exception (and by the way, neither atoi nor std::atoi throw), you might instead simply check for the number of arguments like this:

if (argc < 2) {
    std::cout << "Usage: " << argv[0] << " maxint\n";
    return 0;
}

Provide a reasonable default argument

As you've noted in your comments, a reasonable default would be the product of all of the numbers. That is, 3 5 7 11 13, but that's actually 15015 and not 15016 as your comments indicate. Why not let the computer supply that? Here's one way to do that:

const static int defaultMax = std::accumulate(
        FizzDict.cbegin(), 
        FizzDict.cend(), 
        1, 
        [](int a, const std::pair >& b){ 
            return a*b.first; 
        }
      );
iMax = defaultMax;
if (argc > 1)
    iMax = std::stoi(argv[1]);

for (int i = 1; i <= iMax; ++i)
{
    printString(FizzBuzzer(i, FizzDict));
}

Note that the lambda could be shortened by making the second argument decltype(*FizzDict.cbegin()) b but we still need to know that it's a std::pair so that we can get the first element (integer), so it's not clear that it's really a win.

Separate interface from implementation

The interface goes into a header file and the implementation (that is, everything that actually emits bytes including all functions and data) should be in a separate .cpp file. The reason is that you might have multiple source files including the .h file but only one instance of the corresponding .cpp file. In other words, split your existing FizzBuzz.h file into a .h file and a .cpp file.

Make sure you have all required #includes

The code uses atoi but doesn't #include . Also, carefully consider which #includes are part of the interface (and belong in the .h file) and which are part of the implementation per the above advice.

Encapsulate helper functions within a class

Right now, there's probably little reason to use ifEmpty or checkModZero outside the context of your FizzBuzzer. To that end, you could make those private member functions of a FizzBuzzer class and have your FizzBuzzer function be a public member function instead. I'd probably either have the FizzDict map be a static member of that class or maybe an argument to the constructor.

StackExchange Code Review Q#92188, answer score: 7