31

I'm a total newbie trying to learn C++ from a book. The code below works and produces output as expected, but there are warnings on the two lines that define engine and randomInt: "Initialization of 'engine' with static storage duration may throw an exception that cannot be caught."

If I put lines 7 and 8 inside of main(), the warnings completely go away, but then engine and randomInt are not available to getNumber.

I don't know how to fix the warnings. Also, perhaps more importantly, what is the proper approach for using randomInt in various places besides main()? Is it proper to declare it in main() then pass it to functions as needed? Somehow main() doesn't feel like the proper spot to be declaring these types of things.

I asked a question similar to this one earlier, but I'm still struggling to understand, and have provided an example that's hopefully useful.

// Loosely based on Fig. 6.12: fig06_12.cpp, C++ How To Program, Ninth Edition

#include <iostream>
#include <iomanip>
#include <random>

std::default_random_engine engine( static_cast<unsigned int>( time(nullptr) ) );
std::uniform_int_distribution<unsigned int> randomInt( 1, 6 );

int getNumber();

int main() {
    for ( unsigned int counter = 1; counter <= 10; ++counter ) {
        std::cout << std::setw( 10 ) << randomInt( engine );
        if ( counter % 5 == 0 )
            std::cout << std::endl;
    }
    std::cout << getNumber() << std::endl;
    return 0;
}

int getNumber () {
    return randomInt( engine );
}

Output:

/CLionProjects/Warning/cmake-build-debug/Warning
         3         5         6         3         3
         1         4         2         4         5
2

Process finished with exit code 0
alfC
  • 14,261
  • 4
  • 67
  • 118
Timothy B.
  • 617
  • 1
  • 7
  • 15

3 Answers3

30

One way to defer initialization of global variables such as the ones you are using is to wrap them in get-functions.

std::default_random_engine& getEngine()
{
   // Initialized upon first call to the function.
   static std::default_random_engine engine(static_cast<unsigned int>(time(nullptr)));
   return engine;
}

std::uniform_int_distribution<unsigned int>& getRandomInt()
{ 
   // Initialized upon first call to the function.
   static std::uniform_int_distribution<unsigned int> randomInt(1, 6);
   return randomInt;
}

and then use getEngine() and getRandomInt() instead of using the variables directly.

R Sahu
  • 204,454
  • 14
  • 159
  • 270
  • 10
    Is it considered to be a good practice? As for me, that only makes code more complicated – Enbugger Sep 20 '19 at 11:00
  • 2
    it's good, because the errors occur at function-call time, where exceptions in initialization will propagate to the caller, rather than at module load time. just wrap your global in a function that has a static member... and returns it. – Erik Aronesty Feb 12 '20 at 21:59
  • Wouldn't this lead to initializing the `std::default_random_engine` each time when `getRandomInt()` is called? This feels to me like an unnecessary overhead. Could you please explain that? – Patrick Jun 18 '21 at 11:56
  • 5
    @Patrick No, the std::default_random_engine is declared static, so will only be initialized on first call. – Martin Aug 25 '21 at 03:48
9

Using global variables is problematic, and it is common wisdom to avoid them unless they're absolutely necessary. For details, see:

Are global variables bad?

Your question title also regards non-global-scope static storage duration variables (e.g. static locals of functions); these are less problematic but can also give you some headaches, especially in multi-threaded work.

Bottom line: It's best to make your functions depend only on their parameters and have as few side-effects as is necessary. Let's do this with your getNumber() function:

template <typename Distribution>
typename Distribution::result_type getNumber (
    std::default_random_engine&  random_engine,
    Distribution&                distribution) 
{
    return distribution( random_engine );
}

int main()
{
    std::default_random_engine engine( static_cast<unsigned int>( time(nullptr) ) );
    std::uniform_int_distribution<unsigned int> randomInt( 1, 6 );

    for ( unsigned int counter = 1; counter <= 10; ++counter ) {
        std::cout << std::setw( 10 ) << randomInt( engine );
        if ( counter % 5 == 0 )
            std::cout << std::endl;
    }
    std::cout << getNumber( engine, randomInt ) << std::endl;
    return 0;
}
einpoklum
  • 118,144
  • 57
  • 340
  • 684
  • 1
    Here's my use case: `std::string const kErrorCode = "error_code";`. It seems to me that having a global const string is a perfectly reasonable thing to do. Is there a reasonable way to do it? For now, I'm ignoring the warning message. – Edward Falk Oct 18 '22 at 21:29
  • 1
    `std::string` (typically) allocates space on the heap. You don't need that for a constant string. Use `std::string_view` and make it constexpr; you shouldn't get any warnings. – einpoklum Oct 19 '22 at 07:15
  • string_view is new in C++17, I don't think we're ready to move to C++17 yet at $dayjob, but I'll keep it in mind; it's a good solution. – Edward Falk Oct 20 '22 at 00:47
  • 1
    @EdwardFalk: In that case - get Martin Moene's [string_view lite](https://github.com/martinmoene/string-view-lite/), it's compatible with older versions of C++. – einpoklum Oct 20 '22 at 10:47
1

I agree with the other answers that global objects are problematic.

However, your question is specifically about the initialization of global variables that can throw an exception and, in turn, that there is no C++ scope that can catch this exception (that I know of). (This warning is cert-err58-cpp in the clang-tidy static analyzer.)

Another less know solution that worked in my experience is, if you can control the constructor of the class used as global, to make the constructor(s) invoked noexcept.

I think it has some advantages:

First, this forces you to think if you can avoid doing operations that can fail (e.g. allocation or opening a file)

struct TypeUsedAsGlobal {
    explicit TypeUsedAsGlobal(int n) noexcept {
        ... // body of the constructor that cannot fail
    } catch(...) {}
...
};

and, second, if you cannot avoid them it will force you to handle the error in the constructor itself.

struct TypeUsedAsGlobal {
    explicit TypeUsedAsGlobal(int n) noexcept try : m_{n} {
        ... // body that can fail
    } catch(...) {
        ... // handle the error
        ... // clean up as much as you can
        ... // print a message, and most likely std::terminate
    }
...
};

(try/catch can have a smaller scope I think).

All things considered, this is not that bad, especially if the type is only used for global objects (not in scopes), because well, what can you do if a global object that is (hopefully) fundamental to the whole program cannot be constructed in the first place? (e.g. because a file doesn't exist or if the resource that the global object represent is not there or sufficient to the logic of the program.)

The benefit of this approach relative to the function with a static variable is that your program fails right away and not after the first use of the global object an hour later.

"Fail fast, fail often" in other words.

Of course, it is not clear in general that one approach is better than the other, it depends.


As a final note: it is a bad idea to have a global random number generator. Generators should be created when needed or passed down. It is not safe to call the random number generator/distribution from different threads, but more importantly (mathematically) the reason is that you have to control the lifetime and state of the generators very carefully.

There is no one other to blame here than the authors of the book you are using, notorious for their bad practices.

See here for good alternatives The Definitive C++ Book Guide and List

alfC
  • 14,261
  • 4
  • 67
  • 118