Don’t trust the temporaries (C++)!

Class X is defined as follows (C++):


class X {
    std::map<uint32_t, uint32_t> _map;
public:
    X() {
        for (uint32_t i = 0; i < 20; i++) {
            _map[i] = i + 10;
        }
    }

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

The getTheMap method returns by value.

What is the output of this code?

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

for (; it != et; it++) {
    std::cout << "[" << it->first << ";" << it->second << "]" << std::endl;
}

It’s undefined!

At lines 2 and 3, the map iterators are obtained on a temporary object which is destroyed just at the end of the statement. That’s because getTheMap() returns by value. Consequently, on the for loop at line 5, we are iterating over unallocated memory.

My main concern about class X code was to find a way to stop users using it the wrong way (such as in the above code sample). I was told on stackoverflow that in C++11 the STL itself should have a protection to make that code illegal.

Another interesting and very elegant solution is the use of a wrapper:

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

    class InternalWrapper {
        const std::map<uint32_t, uint32_t>& _internalMap;

    public:
        InternalWrapper(const std::map<uint32_t, uint32_t>& map) :
            _internalMap(map)
        {}

        operator std::map<uint32_t, uint32_t>() const {
            return _internalMap;
        }

    };

public:
    X() {
        for (uint32_t i = 0; i < 20; i++) {
            _map[i] = i + 10;
        }
    }

    InternalWrapper getTheMap() const {
        return InternalWrapper(_map);
    }
};

The magic of this code is done by the automatic type conversion operator at line 12.

Now, the getTheMap() method returns an object of type InternalWrapper by value. This object can be implicitly converted to std::map<uint32_t, uint32_t> type, but it doesn’t have begin() and end() methods, so the following line of code is illegal:

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

The user is now prevented from using the interface incorrectly and the only way he can use it is the correct one, this one:

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();

Wow!