How to create map with keys/values inside class body once (not each time functions from class are called)

Question

I would like to create C++ class which would allow to return value by given key from map, and key by given value. I would like also to keep my predefined map in class content. Methods for getting value or key would be static. How to predefine map statically to prevent creating map each time I call getValue(str) function?

class Mapping
{
  static map<string, string> x;

  Mapping::Mapping()
  {
    x["a"] = "one";
    x["b"] = "two";
    x["c"] = "three";
  }

  string getValue(string key)
  {
    return x[key];
  }

  string getKey(string value)
  {
    map<string, string>::const_iterator it;

    for (it = x.begin(); it < x.end(); ++it)
      if (it->second == value)
        return it->first;

    return "";
  }
};

string other_func(string str)
{
  return Mapping.getValue(str);  // I don't want to do:  new Mapping().getValue(str);
}

Function other_func is called often so I would prefer to use map which is created only once (not each time when other_func is called). Do I have to create instance of Mapping in main() and then use it in other_func (return instance.getValue(str)) or is it possible to define map in class body and use it by static functions?

Answer 1

If you are looking up the value from the key a lot, you will find it easier and more efficient to maintain a second map in parallel with the first.

Answer 2

You don't need to create a static map especially if you ever want to create multiple Mapping objects. You can create the object in main() where you need it, and pass it around by reference, as in:

string other_func(Mapping &mymap, string str)
{
   return mymap.getValue(str);
}

Of course this raises questions about efficiency, with lots of strings being copied, so you might want to just call getValue directly without the extra overhead of calling other_func.

Also, if you know anything about the Boost libraries, then you might want to read up on Boost.Bimap which is sort of what you are implementing here.

http://www.boost.org/doc/libs/1_42_0/libs/bimap/doc/html/index.html

Answer 3

Static is bad. Don't. Also, throw or return NULL pointer on not found, not return empty string. Other_func should be a member method on the object of Mapping, not a static method. This whole thing desperately needs to be an object.

template<typename Key, typename Value> class Mapping {
    std::map<Key, Value> primmap;
    std::map<Value, Key> secmap;
public:
    template<typename Functor> Mapping(Functor f) {
        f(primmap);
        struct helper {
            std::map<Value, Key>* secmapptr;
            void operator()(std::pair<Key, Value>& ref) {
                (*secmapptr)[ref.second] = ref.first;
            }
        };
        helper helpme;
        helpme.secmapptr = &secmap;
        std::for_each(primmap.begin(), primmap.end(), helpme);
    }
    Key& GetKeyFromValue(const Value& v) {
        std::map<Value,Key>::iterator key = secmap.find(v);
        if (key == secmap.end())
            throw std::runtime_error("Value not found!");
        return key->second;
    }
    Value& GetValueFromKey(const Key& k) {
        std::map<Key, Value>::iterator key = primmap.find(v);
        if (key == primmap.end())
            throw std::runtime_error("Key not found!");
        return key->second;
    }
    // Add const appropriately.
};

This code uses a function object to initialize the map, reverses it for you, then provides accessing methods for the contents. As for why you would use such a thing as opposed to a raw pair of std::maps, I don't know.

Looking at some of the code you've written, I'm guessing that you originate from Java. Java has a lot of things that C++ users don't use (unless they don't know the language) like singletons, statics, and such.

Answer 4

Is this what you want?

#include <map>
#include <string>

class Mapping
{
    private:
        // Internally we use a map.
        // But typedef the class so it is easy to refer too.
        // Also if you change the type you only need to do it in one place.
        typedef std::map<std::string, std::string>  MyMap;
        MyMap   x; // The data store.

        // The only copy of the map
        // I dont see any way of modifying so declare it const (unless you want to modify it)
        static const Mapping myMap;

        // Make the constructor private.
        // This class is going to hold the only copy.
        Mapping()
        {
            x["a"]  =       "one";
            x["b"]  =       "two";
            x["c"]  =       "three";
        }


    public:
        // Public interface.
        //    Returns a const reference to the value.
        //    The interface use static methods (means we dont need an instance)
        //    Internally we refer to the only instance.
        static std::string const& getValue(std::string const& value)
        {
            // Use find rather than operator[].
            // This way you dont go inserting garbage into your data store.
            // Also it allows the data store to be const (as operator may modify the data store
            // if the value is not found).

            MyMap::const_iterator   find    = myMap.x.find(value);
            if (find != myMap.x.end())
            {
                // If we find it return the value.
                return find->second;
            }

            // What happens when we don;t find anything.
            // Your original code created a garbage entry and returned that.
            // Could throw an exception or return a temporary reference.
            // Maybe ->  throw int(1);
            return "";
        }

};

Answer 5

First of all, you might want to look up Boost::MultiIndex and/or Boost::bimap. Either will probably help a bit with your situation of wanting to use either one of the paired items to look up the other (bimap is more directly what you want, but if you might need to add a third, fourth, etc. key, then MultiIndex may work better). Alternatively, you might want to just use a pair of sorted vectors. For situations like this where the data remains constant after it's been filled in, these will typically allow faster searching and consume less memory.

From there, (even though you don't have to make it explicit) you can handle initialization of the map object itself a bit like a singleton -- put the data in the first time it's needed, and from then on just use it:

class Mapping { 
    static map<string, string> x;
    static bool inited;
public:
    Mapping() { 
        if (!inited) { 
            x["a"] = "one";
            x["b"] = "two";
            x["c"] = "three";
        }
    }
    string getValue(string const &key) { return x[key]; }
};

// This initialization is redundant, but being explicit doesn't hurt.
bool Mapping::inited = false; 
map<string, string> Mapping::x;

With this your some_func could look something like this:

string some_func(string const &input) {
    return Mapping().getValue(input);
}

This still has a little overhead compared to pre-creating and using an object, but it should be a lot less than re-creating and re-initializing the map (or whatever) every time.