Question

Consider a library exporting a distinct interface for initialization, which has to be called by the user before using anything else the library provides. In this step, certain system state is queried and stored in corresponding variables. This cannot be mapped to constants, but should also not be prone to changes due to writes from external sources, i.e. the system state should be queryable in different translation units but not writable.

One obvious example is a timestamp marking the system startup. This cannot be a compile-time constant, but should also never be writable.

This could be realized with a class implementing only static functions and using private, static members:

// system.h
#include <chrono>
using sys_time_point = std::chrono::system_clock::time_point;

class System
{
public:
  System () = delete;
  // possibly other deleted functions

  static bool                  init    () noexcept;
  static bool                  ready   () noexcept;
  static const sys_time_point& initTime() noexcept;

private:
  static bool           initState_;
  static sys_time_point initTime_;
};

// system.cpp
bool           System::initState_ = false;
sys_time_point System::initTime_  = std::chrono::system_clock::now();

The thing is, I consider the latter approach an inadequate design choice because the functions, although possibly dependent on each other, define more or less miscellaneous functionality for querying system state, not modifying or accessing private state of user-defined type.

I'd much rather go for the second approach. Suppose a namespace System which groups the same functionality as the prior class did

// System.h
#include <chrono>

namespace System
{    
  using sys_time_point = std::chrono::system_clock::time_point;

  bool                  init    () noexcept; // needs to be called
  bool                  ready   () noexcept; // can be used by other lib components and lib clients
  const sys_time_point& initTime() noexcept; // returns system startup time point

  // other stuff here ...
}

// System.cpp
namespace System
{
  namespace 
  {
    bool            sysInitState = false;
    sys_time_point  sysInitTime  = std::chrono::system_clock::now();
  }

  bool init() noexcept
  {
    // init code here ... set and return sysInitState accordingly
  }

  bool ready() noexcept
  {
    return sysInitState;
  }

  const sys_time_point& initTime() noexcept
  {
    return sysInitTime;
  }
}

Using the unnamed namespace, I suppress linking to the variables externally in other translation units. AFAIK, there is no way to access them in other translation units except using the functions in namespace System. Writing is also not possible due to const refs or returns by value - except for the scenario where an evil programmer might const_cast<> the const refs to non-const refs.

My questions would now be:

  • is the above, second solution correct at all?
  • is the above, second solution viable?
  • are there other solutions which might be more safe or easier to implement?

Thanks everyone!

Was it helpful?

Solution

Obscuring the private information in a namespace works, but I recommend putting them in a struct and storing one copy of that struct in a local variable.

// System.cpp
namespace {
    struct SysInit
    {
        bool           state;
        sys_time_point time;

        SysInit()
        : state(false)
        , time (std::chrono::system_clock::now())
        { }

        static SysInit& instance()
        {
            static SysInit rval;
            return rval;
        }
    };
}

void init() noexcept
{
    SysInit::instance().state = true;
}

bool ready() noexcept
{
    return SysInit::instance().state;
}

const sys_time_point& initTime() noexcept
{
    return SysInit::instance().time;
}

The reason for this peculiar trick is there is no order of initialization for globals in different .cpp files. If one of your user's .cpp files calls init() in your example before sysInitTime gets initialized, init may use bad values, or worse, the sysInitTime initializer might change its value, which your library doesn't want.

Static local variables are guaranteed to be initialized once, the first time a function gets called. By storing the data in a static local variable rather than a global, we ensure you have constructed values ready to work with. By putting them in a struct together with one function that returns the whole group, we make it easier on the developer to prove that they are all indeed constructed and up to date (not essential for the algorithm, but it makes it much easier to make sense of the code).

A similar pattern is used by boost, but instead of putting them in an anonymous namespace in a .cpp, they put the variables inside a namespace boost::detail. Boost openly states that if you start mucking around inside boost::detail undefined behavior can occur. They do this because many of their libraries are header only, and don't have a .cpp file to work with. I bring it up because having a detail namespace has become an accepted way of saying "no-touch" to implementation details.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top