Thread: return element in a map

Hello

Whats the right way to return reference to element in a map (in one line):

Code:

class someclass {
public:
  element &get_element(std::string key) { return *(m_map.find(key)); }

private:
  std::map<std::string, element> m_map;
};

Thanks for help!

Originally Posted by Daved

And yet another variation is:

Code:

class someclass {
public:
  element &get_element(std::string key)
  {
    assert(m_map.find(key) != m_map.end());
    return *(m_map.find(key));
  }

private:
  std::map<std::string, element> m_map;
};

An exception is preferable here. Assertions should be fired for programmer errors, not usage errors. They are also compiled out of release builds, usually.

That is why the assert is an option. Based on previous threads from the OP, it is entirely likely that an invalid key is a programmer error and can only happen due to programming error. It is one step better than the "undefined behavior" solution by itself. The fact that the assert is compiled out of release builds is a positive, because that situation should never occur and so there should be no need to waste time checking the iterator against end().

Why will this produce the following error:

Code:

class someclass {
public:
  element &get_element(std::string key) {
    assert(m_map.find(key) != m_map.end());
    return *(m_map.find(key));
  }

private:
  std::map<std::string, element> m_map;
};

Code:

error C2440: 'return' : cannot convert from 'std::pair<_Ty1,_Ty2>' to 'element &'

Thanks again!

Originally Posted by Daved

That is why the assert is an option. Based on previous threads from the OP, it is entirely likely that an invalid key is a programmer error and can only happen due to programming error.

Well, OK, I guess I don't recall any related threads, but this sure looked like a situation where recovery was possible.

l2u's had threads about this over the last couple weeks you can search for if you're curious about the back story. It doesn't really matter for the point at hand though, since we're just discussing potential solutions rather than specifically identifying the "correct" one.

I think the assert is a valid solution even if recovery is possible. Look at vector's operator[] and at() functions. Both present valid solutions to the problem. One uses undefined behavior (potentially with an assert or debug-only error of some kind). The other uses an exception. You use each in different circumstances depending on what you need.

Personally, I would normally prefer a defined solution despite potential performance costs because I'd rather have the code work in all cases than prematurely optimize. However, the implementation cost of catching the out_of_range error in an appropriate place may be significant, and so leaving the behavior undefined might be more appropriate for a situation that should never happen.

With:

Code:

return *(m_map.find(key)->second);

I get error:

Code:

error C2440: 'return' : cannot convert from 'std::string' to 'element &'

Originally Posted by laserlight

hmm... but the original post gives m_map as:

Code:

std::map<std::string, element> m_map;

Clearly, std::string is the key type, element is the mapped type.

I think I know what the problem is.. Class element has * operator overloaded and it returns a string (when I do *element).. So what the solution would be in that case so that I can leave the overloaded operator on the element class?

Thanks!