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!
![]()