Return a reference from a function that takes it as a parameter

Question

I have a class that does this, conceptually. Note that the underlying data types are more complex in the real application. This is just for simplification:

class A
{
  private:
  std::map<std::string, int> database;

  public:
  bool knowsValue(std::string string_id);  // Returns true if string_id is in database
  const int& getValueA(std::string string_id);  // Returns reference to int mapped to string_id in database
}

Because calling getValueA for an unknown string_id causes an error, both commands are usually called together:

if obj.knowsValue(string_id)
  int val = obj.getValueA(string_id)
else
  std::cout << "Value does not exist in database";

Because this requires two subsequent find operations on the database object, I made this function bool getValueB(std::string string_id, int& val), which returns true if string_id is in the database, and assigns the mapped value to val.

The content of the two getValue functions is almost identical, so I wanted to reuse getValueB in getValueA. This is where I am out of my depth, with this attempt:

const int& getValueA2(std::string string_id)
{
  static int val_tmp;  // If not static, this object is deleted after the function call and the reference becomes invalid
  if (getValueB(string_id, val_tmp))
    return static_cast<const int&>(val_tmp);
  else
    return static_cast<const int&>(0);
}

Obviously the static keyword is inappropriate here, because the value should not be shared between functions. Also, the fact that the reference is not const in getValueB is also suboptimal.

My questions:

• What is the right way to write getValueA2, which tries to return a reference that it obtains in a parameter? The intermediate val_tmp seems icky.
• Can the reference be const in this structure at all?

I am leaning towards changing getValueB to const int& getValueB(std::string string_id, const bool value_exists_in_db) to untangle this mess, but I would be interested in finding out what is possible and where I went wrong.

---

edit: Note that the declaration of const int& getValueA(std::string string_id) should not change, ideally, to avoid changes in the rest of the codebase.

Answer 1

I think, fundamentally, you shouldn't be trying to combine these two because they're in fact doing completely different things.

The one argument version (const int& getValueA(const std::string&)) is returning a reference to some value somewhere.

The two argument version (bool getValueA2(const std::string&, int&)) is assigning a copy of the value into a new location provided by the caller.

You have a few options here:

1. Leave the implementations separate.
2. Slightly change the two argument version to something like bool getValueA2(const std::string &, const int *&) which consolidates the functionality. This is pretty ugly and largely unappealing.
3. Do what others are saying and change the signature.
4. Refactor both methods to use a third auxiliary method that actually contains the common functionality.

If you go for #4, it'd probably look something like this (note that I can't make a more relevant example because you haven't given enough details for me to do so):

auto getValueAImpl(const std::string &key) const {
    return database.find(key);
}

const int &getValueA(const std::string &key) {
    auto it = getValueAImpl(key);
    if (it != database.end()) return it->second;
    throw std::runtime_error("key not found");
}

bool getValueA(const std::string &key, int &val) {
    auto it = getValueAImpl(key);
    if (it == database.end()) return false;
    val = it->second;
    return true;
}