1

I have an unordered map std::unordered_map<unsigned int, MyObject*> myDictionary.

When I want to add an entry, I first want to check if the key already exists. If it does, I need to access my object to call a function.

If the key doesn't exists, I want to create a new MyObject with this key.

Is it better to make this ?

MyObject *my_obj;
try
{
   my_obj = myDictionary.at(key);
}
catch (int e)
{
   my_obj = new MyObject();
   myDictionary[key] = my_obj;
}
my_obj->Function();

Or this ?

MyObject *my_obj;
if(myDictionary.find(key) == myDictionary->end())
{
   my_obj = new MyObject();
   myDictionary[key] = my_obj;
}
else
{
   my_obj = myDictionary[key];
}
my_obj->Function();

Or something else ?

Ashkan Mobayen Khiabani
  • 33,575
  • 33
  • 102
  • 171
  • 5
    Please don't let raw pointers own resources. Assuming you can't use normal `MyObject`s in your map, use smart pointers. – chris Jun 20 '14 at 13:44
  • And if you don't know what a smart pointer is... see http://stackoverflow.com/questions/106508/what-is-a-smart-pointer-and-when-should-i-use-one – bwegs Jun 20 '14 at 13:48
  • Try and catch should be used when dealing with exceptions - generally when there is problem with the code - rather than for controlling program flow. I also agree with chris, you should be using a smart pointer there. – finlaybob Jun 20 '14 at 13:50

2 Answers2

2

The best way would be for the map to contain MyObject instead of MyObject *

std::unordered_map<unsigned int, MyObject> myDictionary;

And use it as

myDictionary[key].Function();  // if key doesn't exist, it'll be inserted for you

Assuming you must use MyObject *, use unique_ptr to hold them instead of using raw pointers.

std::unordered_map<unsigned int, std::unique_ptr<MyObject>> myDictionary;

unsigned key = 42;
if(myDictionary.find(key) == myDictionary.end()) {
    myDictionary.insert(std::make_pair(key, std::unique_ptr<MyObject>(new MyObject())));
}
myDictionary[key]->Function();
Praetorian
  • 106,671
  • 19
  • 240
  • 328
2

In your case, I'd go with this:

MyObject*& my_obj= myDictionary[key];

if (my_obj==nullptr)
  my_obj= new MyObject;

my_obj->Function();

This way, you save the double-lookup (one for find and one for insert) into the map that the other methods do when inserting.

As the others said, although this does not specifically concern your question, please use smart pointers:

// Using a std::unordered_map<unsigned int, std::unique_ptr<MyObject>>
std::unique_ptr<MyObject>& my_obj= myDictionary[key];

if (my_obj==nullptr)
  my_obj.reset(new MyObject);

my_obj->Function();
ltjax
  • 15,837
  • 3
  • 39
  • 62
  • Thanks for the voice of sanity, using `find` then `insert` if not found is *exactly* what `[]` is about in the first place. The only case it cannot be used is when the value cannot be default-constructed, in which case `emplace` can be a pretty good deal. – Matthieu M. Jun 20 '14 at 17:29