I recently watched Louis Brandy’s CppCon presentation “Curiously Recurring C++ Bugs at Facebook” on youtube.
For bug#2, which is a well-known trap for STL map-based containers, operator[] will insert the requested key (associated with a default-constructed value) if it is not found.
He mentioned a few workarounds and their disadvantages, like
- use at() method: requires exception handling
- const protect: noobs try to defeat that, transferred to non-const (stripped)
- ban operator[] calls: makes the code ugly
but would like to see something neater. In bug#3, he added that a very common usage is to return a default when the key is not found. The normal approach requires returning a copy of the default (expensive if it’s large), which tempts noobs to return a local reference (to destroyed temporary variables: guaranteed bug).
Considering how much productivity drain a clumsy interface can cause, I think it’s worth spending a few hours of my time approaching it, since I might need to use STL map-based containers myself someday.
Here’s my thought process for the design choices:
- Retain the complete STL interface to minimize user code/documentation changes
- Endow a STL map-based container with a default_value (common use case), so that the new operator[] can return a reference without worrying about temporaries getting destroyed.
- Give users a easy read-only access interface (make intentions clear with little typing)
The code (with detailed comment about design decisions and test cases) can be downloaded here: MapWithDefault. For the experienced, here’s the meat:
#include <unordered_map> #include <map> #include <utility> // std::forward // Legend (for extremely simple generic functions) // =============================================== // K: key // V: value // C: container // B: base (class) template <typename K, typename V, template <typename ... Args> class C = std::map, typename B = C<K,V> > class MapWithDefault : private B { public: // Make default_value mandatory. Everything else follows the requested STL container template<typename... Args> MapWithDefault(V default_value, Args&& ... args) : B(std::forward<Args>(args)...), default_value(default_value) {}; public: using B::operator=; using B::get_allocator; using B::at; using B::operator[]; // Read-only map (const object) uses only read-only operator[] const V& operator[](const K& key) const { auto it = this->find(key); return (it==this->end()) ? default_value : it->second; } using B::begin; using B::cbegin; using B::end; using B::cend; using B::rbegin; using B::crbegin; using B::rend; using B::crend; using B::empty; using B::size; using B::max_size; using B::clear; using B::insert; // using B::insert_or_assign; // C++17 using B::emplace; using B::emplace_hint; using B::erase; using B::swap; using B::count; using B::find; using B::equal_range; using B::lower_bound; using B::upper_bound; public: const V default_value; const MapWithDefault& read_only = static_cast<MapWithDefault&>(*this); };
Note that this is private inheritance (can go without virtual destructors since STL doesn’t have it). I have not exposed all the private members and methods back to public with the ‘using’ keyword yet, but you get the idea.
This is how I normally want the extended container to be used:
int main() { MapWithDefault<string, int> m(17); // Endowed with default of 17 cout << "pull rabbit from m.read_only: " << m.read_only["rabbit"] << endl; // Should read 17 // Demonstrates commonly unwanted behavior of inserting requested key when not found cout << "pull rabbit from m: " << m["rabbit"] << endl; // Should read 0 because the key was inserted (not default anymore) // Won't compile: demonstrate that it's read only // m.read_only["rabbit"] = 42; // Demonstrate writing m["rabbit"] = 42; // Confirms written value cout << "pull rabbit from m_read_only: " << m.read_only["rabbit"] << endl; // Should read 42 cout << "pull rabbit from m: " << m["rabbit"] << endl; // Should read 42 return 0; }
Basically, for read-only operations, always operate directly on the chained ‘m.read_only‘ object reference: it will make sure the const protected version of the methods (including read-only operator[]) is called.
Please let me know if it’s a bad idea or there’s some details I’ve missed!