0

I have a class, whose objects I put into a unordered_set. To do this I have written custom hash generators and comparators to be able to use the class objects in unordered_set. Everything works fine. The comparator of this class looks like this : 

struct MyClassComparator
{
  bool
  operator()(const MyClass & obj1, const MyClass & obj2) const
  {
    if (obj1.getName() == obj2.getName())
      return true;
    return false;
  }
};

So I am comparing the names (strings) of the objects (nothing fancy). I use this to find a MyClass object in a set using .find function.

Now the question is : Is it possible to over load this () operator resulting in the following code 

struct MyClassComparator
{
  bool
  operator()(const MyClass & obj1, const MyClass & obj2) const
  {
    if (obj1.getName() == obj2.getName())
      return true;
    return false;
  }

  bool
  operator()(const MyClass & obj1, const std::string & name) const
  {
    if (obj1.getName() == name)
      return true;
    return false;
  }
};

and use the .find function like 

my_set.find("my_class_name")

if yes is there a performance overhead in doing so.

Hans Olsson
  • 11,123
  • 15
  • 38
AdityaG
  • 428
  • 1
  • 3
  • 17
  • 1
    `if (MyClass.getName() == MyClass.getName())` would appear to always test true? Did you intend `obj1` and `obj2`? – David C. Rankin Feb 26 '18 at 09:07
  • yes i intended it to be obj1 and obj2. Thank you for pointing out I changed it now. – AdityaG Feb 26 '18 at 09:08
  • 3
    what you are trying to create is a transparent comparator which I think `unordered_set` will not make use of – Piotr Skotnicki Feb 26 '18 at 09:10
  • 1
    `std::unordered_set::find` only accepts a `const T&`, so you will find no benefit from overloading `operator()` like you have. You will also require that `MyClass` is implicitly constructible from a `const char[14]`/`const char*` for the call to find to compile. To be fair, you will not have a performance overhead by adding this `operator()` to the overload set, but you will also observe no benefit. – Thomas Russell Feb 26 '18 at 09:11
  • @ThomasRussell so you mean to say that in the call my_set.find("my_class_name"), it will not take the overloaded () into consideration and always use the first one with `const T& ` ? – AdityaG Feb 26 '18 at 09:15
  • how do you pass the comparator to the map? In the constructor? – 463035818_is_not_an_ai Feb 26 '18 at 09:23
  • @user463035818 yes I do. – AdityaG Feb 26 '18 at 09:45
  • would be cool if you add that line, because I have the feeling that the essential part (where the overload is chosen) is missing from your code here. See also [mcve] – 463035818_is_not_an_ai Feb 26 '18 at 09:48

2 Answers2

2

std::unordered_set::find takes a const Key& key argument so if you want to be able to use my_set.find("my_class_name") then MyClass must be constructible from const char[].

This will create a temporary key used as parameter for find. I wouldn't worry about performance at all at this stage. It would be a premature optimization.

I recommend using std::string instead of plain C string:

struct MyClass
{
     MyClass(const std::string& s);
};

and then:

using namespace std::string_literals;
my_set.find("my_class_name"s);

also, as a side note

 if (obj1.getName() == obj2.getName())
      return true;
 return false;

can be rewritten in a simpler way (I recommend it):

return obj1.getName() == obj2.getName();
bolov
  • 72,283
  • 15
  • 145
  • 224
0

If you want to search using data types other than std::unorderd_set::value_type, you want a map, not a set.

class MyClassMap {
   public:
     myclass_map::iterator insert( MyClass& object ) {
        return _map.insert( object.get_Name() );
     }

     myclass_map::iterator find( MyClass& object  ) {
       myclass_map::iterator it = _map.find( object.getName() );
       if( it != _map.end() && it->getName() == object.getName() )
          return it;
       return _map.end();
     }

     myclass_map::iterator find( const std::string& name ) {
       return _map.find(name);
     }


   private:
     typedef std::unordered_map<
         const std::string&,
         MyClass*,
         MyClassHash,
         MyClassComparator
       > myclass_map;

     myclass_map _map;
};
Jorge Bellon
  • 2,901
  • 15
  • 25