Some quick notes about your code:
if(this->addr_map.count(last_name) != 0){
//What can I do here?
You probably wanted it the other way:
if(this->addr_map.count(last_name) == 0){
//handle error
But your real problem lies here:
return addr_map[last_name];
Two things to note here:
- The
operator[] for map can do 2 things: If the element exists, it returns it; If the element doesn't exist, it creaets a new (key,value) pair with the specified key and value's default constructor. Probably not what you wanted. However, if your if statement from before would have been the right way, then the latter would never happen because we would knowthe key exists before hand.
- In calling
count() before, you effectively tell map to try and find the element. By calling operator[], you are telling map to find it again. So, you're doing twice the work to retrieve a single value.
A better (faster) way to do this involves iterators, and the find method:
YourMap::iterator it = addr_map.find(last_name); //find the element (once)
if (it == addr_map.end()) //element not found
{
//handle error
}
return *it.second; //return element
Now, back to the problem at hand. What to do if last_name is not found?
As other answers noted:
- Simplest solution would be to return a pointer (NULL if not found)
- Use
boost::optional.
- Simply return the
YourMap::iterator but it seems that you are trying to "hide" the map from the user of AddressBook so that's probably a bad idea.
throw an exception. But wait, now you'll have to first check that calling this method is 'safe' (or handle the exception when appropriate). This check requires a boolean method like lastNameExists which would have to be called before calling get_by_last_name. Of course then we'er back to square 1. We're performing 2 find operations to retrieve a single value. It's safe, but if you're doing A LOT of calls to get_by_last_name then this is potentially a good place to optimize with a different solution (besides, arguably the exception is not very constructive: What's wrong with searching for something that isn't there, huh?).
- Create a
dummy member for Entryindicating that is not a real Entry but that is very poor design (unmanageable, counter intuitive, wasteful - you name it).
As you can see, the first 2 solutions are by far preferable.