Subtle error when using temporaries to get iterators to a STL container: how to avoid it?

Question

Let's consider this class:

class X {
    std::map<uint32_t, uint32_t> _map;
public:
    X() { /* Populate the map */ }

    std::map<uint32_t, uint32_t> getTheMap() { return _map; }
};

and this buggy code:

X x;
// Statement 1
std::map<uint32_t, uint32_t>::const_iterator it = x.getTheMap().begin();
// Statement 2
std::map<uint32_t, uint32_t>::const_iterator et = x.getTheMap().end();

for (; it != et; it++) {
    /* Access the map using the iterator it */
}

The wrong part is that, in Statement 1 and Statement 2, I'm getting an iterator to a temporary object which is going to be destroyed at the end of each statement. As a result the behavior inside the for() loop is undefined.

A correct usage of the getTheMap() method would have been this:

std::map<uint32_t, uint32_t> map = x.getTheMap();
std::map<uint32_t, uint32_t>::const_iterator it = map.begin();
std::map<uint32_t, uint32_t>::const_iterator et = map.end();

for (/* [...] */)

It must be noted that class X has some serious design issues:

1. _map should be better encapsulated inside the class (both for read and write access), so the getTheMap() method could be avoided
2. If the getTheMap() method is really needed, it could return a reference to _map

However, given class X "as is" (<-- see edit below), is there a way to prevent users from getting the iterator to the temporary?

EDIT: class X can be changed, but getTheMap method should exist and return by value. however I was also thinking about compiler warnings.

Answer 1

One possibility would be to use a wrapper like this:

class X {
  typedef std::map<uint32_t,uint32_t> Map;
  Map _map;

  struct MapWrap {
    const Map &mapref;

    MapWrap(const Map &mapref_arg)
    : mapref(mapref_arg)
    {
    }

    operator Map() const { return mapref; }
  };


public:
  MapWrap getTheMap()
  {
    return MapWrap(_map);
  }
};

so that you get this:

X x;
std::map<uint32_t,uint32_t>::const_iterator iter = x.getTheMap().begin(); // error
std::map<uint32_t,uint32_t> m = x.getTheMap(); // no error

This prevents accidentally using the temporary like a map, but makes it where the user has to use a copy of the map.

Answer 2

Not in C++03. In C++11 the Standard library should already have such protection enabled.

Answer 3

you could try forcing getTheMap() to return the original object by using std::move, but I'm not sure if that will work here.

If not, I guess returning a unique/shared_ptr of the member would be the best option.