Sanity Checking an unordered_set of unique_ptrs to objects I have a use case for a data store containing uniquely identified large objects where I can get partial updates over time. Each object can be identified by a 'key' consisting of a trio of values embedded in different parts of the structure of the object. Yes, this is terrible, and no, I can't change it.

I decided I wanted to use an unordered_set to store my objects because I did not want to have to extract the key to search in a map, and I have a distaste for multiple copies of data, in this case a separate key and the key data embedded in the object.

I have constructed a test case for my needs and it works exactly as I expected. But I am an old dog learning new tricks with respect to smart pointers and wanted some experienced eyes and minds to review it or point out pitfalls my basic test case won't trigger. So without further verbage:

#include &lt;memory&gt;
#include &lt;string&gt;
#include &lt;unordered_set&gt;
#include &lt;list&gt;
#include &lt;iostream&gt;


struct TestRecord
{
   std::string strVal;
   int intVal;
   int intVal2;
   std::string nonKeyStr;
};

using TestPtr = std::unique_ptr&lt;TestRecord&gt;;
using TestList = std::list&lt;TestPtr&gt;;

struct TestPtrHash
{
   std::size_t operator()(TestPtr const&amp; s) const
   {
      if (!s)
      {
         return 0;
      }

      std::size_t h1 = std::hash&lt;std::string&gt;{}(s-&gt;strVal);
      std::size_t h2 = std::hash&lt;int&gt;{}(s-&gt;intVal);
      std::size_t h3 = std::hash&lt;int&gt;{}(s-&gt;intVal2);
      return h1 ^ (h2 &lt;&lt; 1) ^ (h3 &lt;&lt; 2);
   }
};

struct TestPtrEquals
{
   bool operator()(const TestPtr&amp; lhs, const TestPtr&amp; rhs) const
   {
      if (!lhs || !rhs)   
      {
         return false;
      }

      if ((lhs-&gt;strVal == rhs-&gt;strVal) &amp;&amp; (lhs-&gt;intVal == rhs-&gt;intVal) &amp;&amp; (lhs-&gt;intVal2 == rhs-&gt;intVal2))
      {
         return true;
      }

      return false;
   }
};

using TestSet = std::unordered_set&lt;TestPtr, TestPtrHash, TestPtrEquals&gt;;


bool DoThings(const std::string context, const TestPtr&amp; item)
{

   std::cout &lt;&lt; context &lt;&lt; ":" &lt;&lt; item-&gt;strVal &lt;&lt; " " &lt;&lt; item-&gt;intVal &lt;&lt; " " &lt;&lt; item-&gt;intVal2 &lt;&lt; " " &lt;&lt; item-&gt;nonKeyStr &lt;&lt; "\n";

   return true;
}

int main(int argc, char* argv[])
{
   TestList thisList;
   TestSet thisSet;

   thisList.emplace_back(new TestRecord{ "name", 1, 2, "Other" });
   thisList.emplace_back(new TestRecord{ "name2", 1, 2, "Less" });
   thisList.emplace_back(new TestRecord{ "name", 3, 2, "Is" });
   thisList.emplace_back(new TestRecord{ "name", 1, 5, "More" });
   thisList.emplace_back(new TestRecord{ "name2", 1, 2, "More" });


   while (!thisList.empty())
   {
      TestPtr finder(std::move(thisList.front()));
      thisList.pop_front();

      auto look = thisSet.find(finder);

      if (look != thisSet.end())
      {
         (*look)-&gt;nonKeyStr = finder-&gt;nonKeyStr;
         DoThings("replace",*look);
      }
      else
      {
         DoThings("new node",finder);
         thisSet.emplace(std::move(finder));
      }
   }


    return 0;
}
Consider specializing standard templates

If the key fields of your structure are general, wherever it is used (rather than being a subset chosen for this particular set), then you could specialize std::hash and std::equal_to instead of declaring your own functors:

#include &lt;functional&gt;
namespace std
{
    template&lt;&gt;
    std::size_t hash&lt;TestPtr&gt;::operator()(TestPtr const&amp; s) const
    {
        if (!s) return 0;
        auto h1 = std::hash&lt;std::string&gt;{}(s-&gt;strVal);
        auto h2 = std::hash&lt;int&gt;{}(s-&gt;intVal);
        auto h3 = std::hash&lt;int&gt;{}(s-&gt;intVal2);
        return h1 ^ (h2 &lt;&lt; 1) ^ (h3 &lt;&lt; 2);
    }

    template&lt;&gt;
    bool equal_to&lt;TestPtr&gt;::operator()(TestPtr const&amp; lhs, TestPtr const&amp; rhs) const
    {
        return lhs
            &amp;&amp; rhs
            &amp;&amp; lhs-&gt;strVal == rhs-&gt;strVal
            &amp;&amp; lhs-&gt;intVal == rhs-&gt;intVal
            &amp;&amp; lhs-&gt;intVal2 == rhs-&gt;intVal2;
    }
}

using TestSet = std::unordered_set&lt;TestPtr&gt;;


Notes on the test program

I prefer to separate the test code (in Real Life, you'd put it in a separate file):

#include &lt;functional&gt;
#include &lt;memory&gt;
#include &lt;string&gt;
#include &lt;unordered_set&gt;

struct TestRecord
{
   // and the rest of the production code
};

// --- test code follows

#include &lt;iostream&gt;
#include &lt;list&gt;
bool DoThings(const std::string context, const TestPtr&amp; item)
{
   //
   return true;
}

int main()
{
   // Note: omitted arguments, which weren't used
}


When you move the production code to a header, you don't have to strip out the unnecessary includes.

The return value of DoThings() is ignored; either use it, or return void.

When inserting, it's more idiomatic to call insert() rather than a separate find() step.  The return value tells you whether your value was inserted - if it wasn't, you also get an iterator to assign to.  However, as you've observed, you aren't able to perform the assignment if you passed your value by rvalue reference.  Still, I'd try to encapsulate this in a method:

auto insert_or_assign(TestSet&amp; set, TestPtr&amp;&amp; value)
    -&gt; std::pair&lt;TestSet::iterator, bool&gt;
{
    auto overwrite = set.erase(value);
    auto r = set.insert(std::move(value));
    r.second = !overwrite;
    return r;
}


You can then use it like this:

int main()
{
    std::vector&lt;TestPtr&gt; thisList;

    thisList.emplace_back(new TestRecord{ "name", 1, 2, "Other" });
    thisList.emplace_back(new TestRecord{ "name2", 1, 2, "Less" });
    thisList.emplace_back(new TestRecord{ "name", 3, 2, "Is" });
    thisList.emplace_back(new TestRecord{ "name", 1, 5, "More" });
    thisList.emplace_back(new TestRecord{ "name2", 1, 2, "More" });

    TestSet thisSet;
    for (auto&amp; t: thisList) {
        auto inserted = insert_or_assign(thisSet, std::move(t));
        DoThings(inserted.second ? "new node" : "replace", *inserted.first);
    }
}