CODEWITHSUNDEEP
c++
Roko
Roko

Reputation: 1335

C++: std::map<int,int> crashes when being assigned

I'm trying to make an int to int map but the program crashes and I don't realize why. I've summarized the problem to this short simple code.

When the program starts, since the Utilities member _instance is static- it initialize it, by going to its constructor, which contain a single line: An int to int (simple) map assignment. but then it crashes.

Notice that if I comment that line, the program doesn't crash, and that the main contains that very same line. So my two questions are:

1) Why does it crash? is there a point behind this behavior?

2) How do I fix it so I can initialize the map at constructor?

Thank You

#include <map>

class Utilities
{
public:

    ~Utilities(){};
    static Utilities& instance();

private:
    Utilities();
    Utilities( const Utilities& ){};

    static Utilities _instance;
    static std::map<int, int> textIntToIntMap;
};


Utilities Utilities::_instance = Utilities();
std::map<int, int> Utilities::textIntToIntMap;

Utilities::Utilities()
{
    //The following line crashes, why?
    textIntToIntMap[1] = 2;
}

int main()
{
    static std::map<int, int> text2;
    text2[4] = 2;
    int xxx = 3;
}

Upvotes: 2

Views: 3207

Answers (5)

Surj
Surj

Reputation: 1

Try this way::

#include <map>
#include <iostream>
using namespace std;

class Utilities
{
public:
    static Utilities& instance() {
        static Utilities instance;
        return instance;
    }
    ~Utilities(){};
    void PrintMapValues();
    void AddKeyValue(int key, int value);

private:
    Utilities();
    Utilities( const Utilities& ){};

    std::map<int, int> int_to_int_map_;
};

Utilities::Utilities()
{
    //The following line crashes, why?
    int_to_int_map_[-99] = 2;
}

void Utilities::PrintMapValues() {
    for(std::map<int, int>::iterator it = int_to_int_map_.begin(); it != int_to_int_map_.end(); ++it){
        cout << "Key:" << it->first << " Val:" << it->second << endl;
    }
}

void Utilities::AddKeyValue(int key, int value) {
    int_to_int_map_[key] = value;
}

int main()
{
    Utilities& utils = Utilities::instance();

    for (int i=0; i< 10; i++) {
        utils.AddKeyValue(i, i+20);
    }

    utils.PrintMapValues();

    return 0;
}

Upvotes: 0

Kuba hasn&#39;t forgotten Monica
Kuba hasn&#39;t forgotten Monica

Reputation: 98425

Others have already caught that the Utilities() are constructed before Utilities::textToIntMap. So, now the question is: how to avoid such problems in the future?

You can use a function returning a reference to a static local variable to ensure that the construction is done before the reference is used. You can put such function into a namespace. You should also have a convenience typedef with hopefully a shorter name, so that declaring iterators is not a finger-breaker on C++98. On C++11 you should be using auto anyway.

Note that the use of global or function-static non-POD data is not thread-safe in C++98. If you wish to safely use textToMap() from multiple threads, with no guarantee that it was accessed before the 2nd thread was started, textToIntMap would need to wrap the initialization within a mutex. To get a rough idea how this might be done, see the inner function from Qt's convenient Q_GLOBAL_STATIC.

The use of a singleton class seems like a pointless Java-ism in this case.

Run a test on ideone.

// Utilities.h
namespace Utilities {
  typedef std::map<int, int> Map;
  Map & textToIntMap();
}

// Utilities.cpp
namespace Utilities {
  namespace {
    struct InitializedMap : Map {
      InitializedMap() {
         insert(value_type(1, 2));
         // or
         (*this)[1] = 2;
      }
    };
  }
  Map & textToIntMap() {
    static InitializedMap map;
    return map;
  }
}

Upvotes: 0

user2064126
user2064126

Reputation: 1

Before initializing _instance you need to define map textIntToIntMap outside the class. Because in constructor you are using textIntToIntMap which is a static member so you need to define it first. So outside the class use following lines:

std::map<int, int> Utilities::textIntToIntMap;
Utilities Utilities::_instance = Utilities();

Upvotes: 0

juanchopanza
juanchopanza

Reputation: 227418

You have an initialization order problem:

Utilities Utilities::_instance = Utilities();

This line calls the default constructor of Utilities, which then tries to populate the map. But the map is not initialized at this stage.

You should design your code to be robust against such initialization order issues. You can mitigate some of that by creating static instances inside functions. This gives you a handle on the ordering of initializations. But a simple re-ordering of the definitions should fix the immediate problem:

std::map<int, int> Utilities::textIntToIntMap;
Utilities Utilities::_instance = Utilities();  // OK, map has been defined

Upvotes: 3

NPE
NPE

Reputation: 500357

The issue is that you call the Utilities constructor before Utilities::textIntToIntMap has been constructed.

Swap the order of the following two lines:

Utilities Utilities::_instance = Utilities();
std::map<int, int> Utilities::textIntToIntMap;

Upvotes: 2

Related Questions