2

I have a class which contains a static member, a map of strings to function pointers. This map is intended to be populated once with a static set of mappings, and will not subsequently be modified.

My question is, how can I ensure the map is not accessed before it is initialised? My code currently looks something like this:

class MyClass
{
  static MapType s_myMap;
public:
  static const MapType& getTheMap()
  {
    if (s_myMap.empty())
    {
      // Populate the map
    }
    return s_myMap;
  }
};

which works fine for external clients of MyClass, but doesn't prevent internal class members from directly accessing the private map before it has been initialised.

To address that problem I am thinking of making the map local to the getter method:

class MyClass
{
public:
  static const MapType& getTheMap()
  {
    static MapType s_myMap;
    if (s_myMap.empty())
    {
      // Populate the map
    }
    return s_myMap;
  }
};

Is that a good idea, or is there a better way of achieving this?

atkins
  • 1,963
  • 14
  • 27
  • Moving the static to the function is a good idea. As long as no other static object initialization ever calls `getTheMap`, you're fine. – Mooing Duck Nov 01 '11 at 16:30
  • Is the map dynamically initialized? If not, you could just populate it in the initializer (provided you have a recent compiler). – Kerrek SB Nov 01 '11 at 17:06
  • @KerrekSB It's statically initialised. Just to check I've understood you, are you suggesting moving the code implied by `// Populate the map` to a separate initialisation method (say `initialiseMap()`) and then initialising the map via `MapType MyClass::s_myMap = initialiseMap()`? – atkins Nov 01 '11 at 17:14
  • @atkins: I would just write `const MapType MyClass::s_myMap{ { 1, "Jim"}, { 2, "Jane" }, { 3, "Joseph" } };` as the initializer and not have a separate function to perform the population. – Kerrek SB Nov 01 '11 at 17:24
  • @KerrekSB Alas, it looks like my compiler doesn't support this syntax (we're stuck on MSVC 2003). I get `error C2470: 'MyClass::s_myMap' : looks like a function definition, but there is no formal parameter list; skipping apparent body`. Thanks for the idea though! – atkins Nov 01 '11 at 17:39
  • @Mooing Duck: What is wrong with other static code calling this function. Looks good to me. – Martin York Nov 01 '11 at 18:18
  • @LokiAstari: If a static object's initializer in another translation unit calls `getTheMap()` _before_ s_myMap has been initialized, that's undefined behavior. If it's inside `getTheMap`, that's not an issue. – Mooing Duck Nov 01 '11 at 19:08
  • @Mooing Duck: No its not. Its very well defined. The static function variable `s_myMap` will be initialized the first time the method `getTheMap()` is called before any other processing is done. The fact that it is called from the constructor of another static storage duration object (say p) is irrelevant. Apart from to the fact that s_myMap is now guaranteed to by fully constructed first (ie before p) and thus available to the other object (p) and will also be destroyed after the object (p) is destroyed and thus available in the destructor of the other object(p). – Martin York Nov 01 '11 at 21:27
  • @LokiAstari: C++11 (Feb draft) § 3.6.2/4: `[Example with two non-local variables with static duration a and b] ...if a is initialized before main is entered, it is not guaranteed that b will be initialized before it is odr-used by the initialization of a, that is, before A::A is called.` See: http://www.parashift.com/c++-faq-lite/ctors.html#faq-10.14 – Mooing Duck Nov 01 '11 at 21:46
  • For clarification, I'm talking about the problem when the static is not in the function. Where it's inside the function this does not apply. – Mooing Duck Nov 01 '11 at 22:00
  • @Mooing Duck: Your quote is correct. But not relevant here. As theses are static storage duration members **inside of a a function** (i.e. they are local and your reference is about non local objects). The order of initialization is well defined. It is at the point that the function is called. See `6.7 Declaration statement paragraph 4` – Martin York Nov 01 '11 at 22:12
  • @Mooing Duck: If you are talking about 'local objects' then your first comment above does not make sense. PS. I hate parashift.com it is riddled with subtle errors (Can you see the error here? http://www.parashift.com/c++-faq-lite/dtors.html#faq-11.10 (though he is slowly fixing them)). Stick to quoting the standard. – Martin York Nov 01 '11 at 22:14
  • @LokiAstari: It sounds like we're in agreement to me. His first code, where the static is outside of the function, might cause problems. I agree with you that the second code, where the static is _inside_ the function, is perfectly fine. – Mooing Duck Nov 01 '11 at 22:20

4 Answers4

1

You can declare a class for this collection and populate it in constructor.

Michael Krelin - hacker
  • 138,757
  • 24
  • 193
  • 173
1

If MyClass::getTheMap() is not called in global / namespace scope, then you don't have to worry about the static data member being used before it's initialized.

However if, the above static method getTheMap() is used in global / namespace scope:

SomeGlobal object = MyClass::getTheMap();

then your current approach seems to be fine.

iammilind
  • 68,093
  • 33
  • 169
  • 336
  • I've been a bit loose with the word "initialise". What I meant by it here is that the map should be populated with a set of entries before it is accessed. The data member is initialised in the more precise sense of the word as an empty map using `MapType MyClass::s_myMap = MapType()`. – atkins Nov 01 '11 at 16:52
  • @atkins: If you haven't wandered into undefined behavior first, then `getTheMap()` will correctly set the entries. The problem is it's _really_ easy to wander into undefined behavior with global static variables. – Mooing Duck Nov 01 '11 at 19:12
1

Moving the static into the function will solve any order of initialization problems, but it may leave you with an order of destruction one. In many cases, it's preferable to use a pointer and dynamic allocation, so that the map is never destructed.

As for initializing it, I often use the two iterator constructor, so that I can make the map itself const. To do this, just define a struct with a conversion operator, something like:

struct MapInitData
{
    char const* key;      //  Or whatever type is needed.
    char const* value;    //  Or whatever type is needed.
    operator MapType::value_type() const
    {
        return MapType::value_type( key, value );
    }
};

MapInitData const mapInitTable[] =
{
    { "key1", "value1" },
    //  ...
};

MapType const ourMap( begin( mapInitTable ), end( mapInitTable ) );
James Kanze
  • 150,581
  • 18
  • 184
  • 329
  • That's great - I really wanted to make the map `const` but didn't think I'd be able to. – atkins Nov 01 '11 at 17:16
  • I hate this advice (about pointer and dynamic allocation). The order of destruction problem is easy to solve. http://stackoverflow.com/questions/335369/finding-c-static-initialization-order-problems/335746#335746. – Martin York Nov 01 '11 at 17:53
  • @LokiAstari I don't see where the code there solves anything. The order of destruction is only ordered within specific subsets of objects, like those declared statically. Suppose you have a `static std::auto_ptr`, and long after its construction, you reset it to point to an object which uses this static object in its destructor. (Such cases aren't common---hopefully, at least. But the are possible.) – James Kanze Nov 01 '11 at 18:28
  • @James Kanze: The order of destruction `of static storage duration objects` is **guaranteed** to be the inverse of the order of construction. I don't see how your example (in the comments) is relevant at all. – Martin York Nov 01 '11 at 18:37
  • @LokiAstari The order of destruction of objects with static storage duration is guaranteed. But not all objects have static storage duration. Consider something like 'static auto_ptr p; static Obj o; int main() { p = auto_ptr new T( &o ); }`. What happens if `T` uses the argument passed into it in its destructor? – James Kanze Nov 01 '11 at 18:41
  • @JamesKanze: If you read the link I provided it explains how you can guarantee that the order is correct. The problem here is the technique you are suggesting (which is what I saying don't use) that is actually causing the problem. So your suggestion is causing the problem that I am crying out against and you are using your suggestion as an explanation of why you need to use it. If you don't do this (don't use dynamic memory like this) the problem does not exist. My conclusion is that this technique is causing the problem. SO just don't do it. – Martin York Nov 01 '11 at 19:50
  • @LokiAstari I read the link, and it doesn't solve the problem. And the problem is real. – James Kanze Nov 02 '11 at 09:40
  • @James Kanze: If you introduce a technique that has a problem with it then that is a good reason not to use the technique. The order of destruction problem is easily solved (as described in the link). If you want to add a bad technique that can cause a problem then so be it. That does not make the order of destruction a problem it makes the technique that you introduced the problem. – Martin York Nov 02 '11 at 16:06
  • @LokiAstari The link contained misstatements which don't correspond to the way C++ actually works. The order of destruction problem is easily solved by the solution I propose here: not destructing objects which may be needed in destructors. I don't know why you are trying to complicate the issue. – James Kanze Nov 02 '11 at 16:54
  • @James Kanze: Please point out which statements you think are wrong. I will point you at the section of the standard were each statement is guaranteed. We will just have to disagree on the best technique. I think your solution just makes the code more complicated and causes problems. You introduce a problem that is only fixed by using the problem-ed technique is a problem in my opinion. And by the number of votes you got I don't think many people agree with you. – Martin York Nov 02 '11 at 17:41
  • @LokiAstari "Remember the order of destruction is the exact inverse of the order of construction." That is quite simply false; dynamically allocated objects can be destructed in any order, for example. I posted an earlier an example where the order of destruction wasn't the same as the order of construction. And of course, the number of votes has nothing to do with correctness; the voting here is more a popularity contest, with no real relationship to the value of the articles. – James Kanze Nov 02 '11 at 18:42
  • @JamesKanze: Your manipulation of the information just does not hold. The order of destruction is guaranteed to be the inverse of the order of construction for **static storage duration objects** (which is what the article is about). It is **your introduction** of dynamic storage duration objects into this equation is the whole problem that is causing the problem. Yes there is some linking to popularity over the short term but over the long term the correct answer will float to the top. This answer is just so obviously wrong though. – Martin York Nov 02 '11 at 19:36
  • @LokiAstari You're just ignoring the facts. I actually posted an example where the solution you posted didn't work. – James Kanze Nov 03 '11 at 08:59
  • @JamesKanze: You created a problem who's solution is using the problem repeatedly to solve the problem you just created. If you don't use the technique (which is what I said in my first comment) then the problem does not exist to be fixed. You are just deluding yourself. Still no votes I see. – Martin York Nov 03 '11 at 09:47
  • @LokiAstari I created a problem by using the frequent and useful idiom of lazy initialization. If I don't use any dynamically allocated objects, then I can avoid the problem (at some extra effort). Or I can just use the standard work-around, using a pointer in the singleton so that it never gets destructed---both simpler and safer than any of the alternatives. (And who the hell cares about votes.) – James Kanze Nov 03 '11 at 10:41
  • @James Kanze: Actually I disagree with your summation. The easier safer and know to work idium of lazy initialization can easily be done without using dynamically allocated objects. Its called a static member of a function. There is never any need to use dynamic memory for any of these problems and it is the clasic anti-pattern. – Martin York Nov 03 '11 at 10:46
0

your function 'getTheMap' should have a lock in it for thread safety. Moving it local to the function is a good idea.

I think that this is related to the drdobbs article "C++ and The Perils of Double-Checked Locking" http://drdobbs.com/cpp/184405726 is similar to the question. Here he goes through use cases and safety of different types of singleton & other patterns with threadsafety.

as far as destruction, do you need to destroy it?

Michael
  • 3,604
  • 3
  • 19
  • 12