CodeSOD: An Exert Operation

The Standard Template Library for C++ is… interesting. A generic set of data structures and algorithms was a pretty potent idea. In practice, early implementations left a lot to be desired. Because the STL is a core part of C++ at this point, and widely used, it also means that it’s slow to change, and each change needs to go through a long approval process.

Which is why the STL didn’t have a std::map::containsfunction until the C++20 standard. There were other options. For example, one could usestd::map::count, to count how many times a key appear. Or you could use std::map::findto search for a key. One argument against adding astd::map::containsfunction is thatstd::map::count basically does the same job and has the same performance.

None of this stopped people from adding their own. Which brings us to Gaetan‘s submission. Absent a std::map::contains method, someone wrote a whole slew of fieldExists methods, where field is one of many possible keys they might expect in the map.

<span class="hljs-function"><span class="hljs-type">bool</span> <span class="hljs-title">DataManager::thingyExists</span> <span class="hljs-params">(string name)</span>
</span>{
    THINGY* l_pTHINGY = (*m_pTHINGY)[name];
    <span class="hljs-keyword">if</span>(l_pTHINGY == <span class="hljs-literal">NULL</span>)
    {
        m_pTHINGY-><span class="hljs-built_in">erase</span>(name);
        <span class="hljs-keyword">return</span> <span class="hljs-literal">false</span>;
    }
        <span class="hljs-keyword">else</span>
    {
        <span class="hljs-keyword">return</span> <span class="hljs-literal">true</span>;
    }
    <span class="hljs-keyword">return</span> <span class="hljs-literal">false</span>;
}

I’ve head of upsert operations- an update and insert as the same operation, but this is the first exert- an existence check and an insert in the same operation.

“thingy” here is anonymization. The DataManager contained several of these methods, which did the same thing, but checked a different member variable. Other classes, similar to DataManager had their own implementations. In truth, the original developer did a lot of “it’s a class, but everything inside of it is stored in a map, that’s more flexible!”

In any case, this code starts by using the [] accessor on a member variable m_pTHINGY. This operator returns a reference to what’s stored at that key, or if the key doesn’t exist inserts a default-constructed instance of whatever the map contains.

What the map contains, in this case, is a pointer to a THINGY, so the default construction of a pointer would be null- and that’s what they check. If the value is null, then we erase the key we just inserted and return false. Otherwise, we return true. Otherotherwise, we return false.

As a fun bonus, if someone intentionally stored a null in the map, this will think the key doesn’t exist and as a side effect, remove it.

Gaetan writes:

What bugs me most is the final, useless return.

I’ll be honest, what bugs me most is the Hungarian notation on local variables. But I’m long established as a Hungarian notation hater.

This code at least works, which compared to some bad C++, puts it on a pretty high level of quality. And it even has some upshots, according to Gaetan:

On the bright side: I have obtained easy performance boosts by performing that kind of cleanup lately in that particular codebase.

Source: Read More