14

For an std::map<std::string, std::string> variables, I'd like to do this:

BOOST_CHECK_EQUAL(variables["a"], "b");

The only problem is, in this context variables is const, so operator[] won't work :(

Now, there are several workarounds to this; casting away the const, using variables.count("a") ? variables.find("a")->second : std::string() or even making a function wrapping that. None of these seem to me to be as nice as operator[]. What should I do? Is there a standard way of doing this (beautifully)?

Edit: Just to state the answer that none of you want to give: No, there is no convenient, beautiful, standard way of doing this in C++. I will have to implement a support function.

Magnus Hoff
  • 21,529
  • 9
  • 63
  • 82

7 Answers7

11
template <typename K, typename V>
V get(std::map<K, V> const& map, K const& key)
{
    std::map<K, V>::const_iterator iter(map.find(key));
    return iter != map.end() ? iter->second : V();
}

Improved implementation based on comments:

template <typename T>
typename T::mapped_type get(T const& map, typename T::key_type const& key)
{
    typename T::const_iterator iter(map.find(key));
    return iter != map.end() ? iter->second : typename T::mapped_type();
}
C. K. Young
  • 219,335
  • 46
  • 382
  • 435
  • I am going for this implementation, putting it in a support library so we don't need to reimplement it everywhere :) I'd love it if STL focused more on good old convenience out of the box... Thanks :) – Magnus Hoff Sep 30 '08 at 11:53
  • Do take a look at janm's solution, too! his comparison function won't return true for a key that's not in the map in combination with a default comparand! – xtofl Sep 30 '08 at 11:55
  • BTW, the only reason why the function returns V, and not V const&, is because if returning the default V(), we don't want to return a dangling reference. If you are prepared to guarantee that the item exists, simply do "return iter->second;" (undefined behaviour if item doesn't exist). – C. K. Young Sep 30 '08 at 11:57
  • I think the get-function might be applicable outside of just testing for equality. I do recognise the weakness with the default value. – Magnus Hoff Sep 30 '08 at 12:02
  • It seems to work better when I add another template-parameter, K2 as such: `template V get(std::map const& map, K2 const& key)` This way, I can use a string literal for the key parameter and have it implicitly converted to an `std::string`. – Magnus Hoff Sep 30 '08 at 13:34
  • This can be expressed so that it works with hash_map and other containers (and no K1/K2): template typename CONT::mapped_type get(const CONT& m, const typename CONT::key_type& k) { CONT::const_iterator i(m.find(k)); return i == m.end() ? CONT::mapped_type() : i->second; } – janm Sep 30 '08 at 14:46
  • Just noticed that Matt Price made the same observation in one of the answers. Of course I agree with his observations! – janm Sep 30 '08 at 14:50
  • janm: Agree! I think your answer or Matt's one should be accepted instead, however, in the meantime I will edit my entry to take these into account. :-) – C. K. Young Sep 30 '08 at 21:03
  • I've tested my implementation with both std::map and std::tr1::unordered_map and they both work. Of course I was super-pedantic and put "typename" across everything that required it. :-P – C. K. Young Sep 30 '08 at 21:13
11

Casting away const is wrong, because operator[] on map<> will create the entry if it isn't present with a default constructed string. If the map is actually in immutable storage then it will fail. This must be so because operator[] returns a non-const reference to allow assignment. (eg. m[1] = 2)

A quick free function to implement the comparison:

template<typename CONT>
bool check_equal(const CONT& m, const typename CONT::key_type& k,
                    const typename CONT::mapped_type& v)
{
    CONT::const_iterator i(m.find(k));
    if (i == m.end()) return false;
    return i->second == v;
}

I'll think about syntactic sugar and update if I think of something.

...

The immediate syntactic sugar involved a free function that does a map<>::find() and returns a special class that wraps map<>::const_iterator, and then has overloaded operator==() and operator!=() to allow comparison with the mapped type. So you can do something like:

if (nonmutating_get(m, "key") == "value") { ... }

I'm not convinced that is much better than:

if (check_equal(m, "key", "value")) { ... }

And it is certainly much more complex and what is going on is much less obvious.

The purpose of the object wrapping the iterator is to stop having default constructed data objects. If you don't care, then just use the "get" answer.

In response to the comment about the get being preferred over a comparison in the hope of finding some future use, I have these comments:

  • Say what you mean: calling a function called "check_equal" makes it clear that you are doing an equality comparison without object creation.

  • I recommend only implementing functionality once you have a need. Doing something before then is often a mistake.

  • Depending on the situation, a default constructor might have side-effects. If you are comparing, why do anything extra?

  • The SQL argument: NULL is not equivalent to an empty string. Is the absence of a key from your container really the same as the key being present in your container with a default constructed value?

Having said all that, a default constructed object is equivalent to using map<>::operator[] on a non-const container. And perhaps you have a current requirement for a get function that returns a default constructed object; I know I have had that requirement in the past.

janm
  • 17,976
  • 1
  • 43
  • 61
  • This is a good idea, but I prefer the get-implementation (even with its weakness) in hopes that the function might be useable outside of testing for equality. – Magnus Hoff Sep 30 '08 at 12:00
  • janm: Indeed, I like your solution best. I agree about the NULL argument, too. You can, for example, say check_equal("foo", "") and have it return true only if map["foo"] exists and is empty. get(map, "foo") == "" says something different. – C. K. Young Sep 30 '08 at 12:39
  • You give good advice in general for coding C++, thanks. I am really rather looking for how to read from a const map than how to compare a value to one in a const map. For the code base I am working in, I am confident that the get function will be a nice addition. Thanks again, though :) – Magnus Hoff Sep 30 '08 at 12:42
5

find is the idiomatic form. Casting away const is almost always a bad idea. You'd have to guarantee that no write operation is performed. While this can be reasonably expected of a read access on a map, the specification doesn't say anything about this.

If you know that the value exists you can of course forego the test using count (which is quite inefficient, anyway, since it means traversing the map twice. Even if you don't know whether the element exists I wouldn't use this. Use the following instead:

T const& item(map<TKey, T> const& m, TKey const& key, T const& def = T()) {
    map<TKey, T>::const_iterator i = m.find(key);
    return i == m.end() ? def : i->second;
}

/EDIT: As Chris has correctly pointed out, default-construction of objects of type T might be expensive, especially since this is done even if this object isn't actually needed (because the entry exists). If this is the case, don't use the default value for the def argument in the above case.

Konrad Rudolph
  • 530,221
  • 131
  • 937
  • 1,214
  • Ow! That means default-constructing a string for every call, when `default` (a keyword, by the way) is not specified. – C. K. Young Sep 30 '08 at 11:42
  • Default-constructions shouldn't be too expensive for most types. If they are – don't use the default argument. – Konrad Rudolph Sep 30 '08 at 11:51
  • That, or have overloaded versions, one with your version (but no default value), and one with mine. :-) – C. K. Young Sep 30 '08 at 11:53
  • My above suggestion is overkill for the question, mind you. :-P – C. K. Young Sep 30 '08 at 11:53
  • I think your solution does have one benefit: you can return by T const& (useful if T is expensive to copy, for example). So yes, I've upvoted your entry too (aside from its greater generality). :-) – C. K. Young Sep 30 '08 at 12:01
5

An interesting aside, there are two ways do the template type discovery in the get implementation that was accepted (the one that gets the value or returns a default constructed object). One, you can do what was accepted and have:

template <typename K, typename V>
V get1(const std::map<K, V>& theMap, const K const key)
{
    std::map<K, V>::const_iterator iter(theMap.find(key));
    return iter != theMap.end() ? iter->second : V();
}

or you can use the map type and get the types off of that:

template<typename T>
typename T::mapped_type
get2(const T& theMap, const typename T::key_type& key)
{
    typename T::const_iterator itr = theMap.find(key);
    return itr != theMap.end() ? itr->second : typename T::mapped_type();
}

The advantage of this is that the type of the key being passed in doesn't play in the type discovery and it can be something that can be implicitly converted to a key. For example:

std::map<std::string, int> data;
get1(data, "hey"); // doesn't compile because the key type is ambiguous
get2(data, "hey"); // just fine, a const char* can be converted to a string
Matt Price
  • 43,887
  • 9
  • 38
  • 44
  • I've edited my answer, due to your argument, and to janm's one about being container-agnostic. Incidentally the end result is a lot like yours (I wrote my own implementation, but I guess there just aren't _that_ many ways to write it). :-P – C. K. Young Sep 30 '08 at 21:16
1

Indeed, operator[] is a non-const one on std::map, since it automatically inserts a key-value pair in the map if it weren't there. (Oooh side-effects!)

The right way is using map::find and, if the returned iterator is valid (!= map.end()), returning the second, as you showed.

map<int, int> m;
m[1]=5; m[2]=6; // fill in some couples
...
map<int,int>::const_iterator it = m.find( 3 );
if( it != m.end() ) {
    int value = it->second;
    // ... do stuff with value
}

You could add a map::operator[]( const key_type& key ) const in a subclass of the std::map you're using, and assert the key to be found, after which you return the it->second.

xtofl
  • 40,723
  • 12
  • 105
  • 192
0

Following up xtofl's idea of specializing the map container. Will the following work well?

template <typename K,typename V>  
struct Dictionary:public std::map<K,V>  
{  
  const V& operator[] (const K& key) const  
  {  
    std::map<K,V>::const_iterator iter(this->find(key));  
    BOOST_VERIFY(iter!=this->end());  
    return iter->second;  
  }  
};  
0
std::map<std::string, std::string>::const_iterator it( m.find("a") );
BOOST_CHECK_EQUAL( 
                     ( it == m.end() ? std::string("") : it->second ), 
                     "b" 
                 );

That doesn't look too bad to me... I probably wouldn't write a function for this.

oz10
  • 153,307
  • 27
  • 93
  • 128