Should I pass a string by value or pass a pointer to it?

Question

I have a function which looks like this

int myclass::setVersion(std::string ver)
{
  if (ver.size()>1)
  {
    version.swap(ver)
    return 0;
  }
  else 
    return -1;
}

My question is very simple, is it better to pass ver the way it is or better to pass it as a pointer to string? FYI, ver is a small size string (around 8). EDIT: it does not matter if ver is changed. I just want to replace version with the contents of ver. EDIT2: I am using visual studio 2008.

Answer 1

It depends on what you want the behavior of the code to be.

Most of the suggested answers here change the behavior from your posted code in that they will modify the string that's passed in (or make the code fail to compile because it can't modify the passed in argument).

In the example you posted,the string passed to myclass::setVersion() will not be modified (the parameter may be modified, but that is just a copy fo the string passed in; a copy which will be destroyed when the function returns).

For a a case like this, I'd suggest passing a const std::string&:

int myclass::setVersion(std::string const& ver)
{
  if (ver.size()>1)
  {
    version = ver;
    return 0;
  }
  else 
    return -1;
}

This way the copy is made only when necessary.

But honestly, unless the function is called often, it's probably nothing to worry much about.

Answer 2

In C++11 you can add an overload which accepts rvalue reference:

int myclass::setVersion(std::string& ver);
int myclass::setVersion(std::string&& ver);

This way you'll be able to swap from rvalues as well as lvalues. And actually instead of swap you can perform a move assignment:

version = std::move(ver);

This is potentially faster than swap, depending on std::string implementation.

Examples of usage:

string getVersion() { return "version2"; }

string v1 = "version1";
a.setVersion(v1);           // lvalue
a.setVersion(getVersion()); // rvalue
a.setVersion("version3");   // rvalue as well

Demo

UPDATE: Indeed, you have to have both variants of setVersion for maximum flexibility. But I agree with others in that all this is premature optimization and should be used only if profiling shows a bottleneck at this point.

Answer 3

Please pass a reference.

int myclass::setVersion(std::string& ver)

Answer 4

You want to store the contents of ver into version. A copy has to be made, the question is where.

You can do it the way you propose, as swapping is fast. But I notice not all control paths cause the version string to be assigned a new value.

Therefore I suggest the following

int myclass::setVersion(const std::string& ver)
{
  if (ver.size()>1)
  {
    version = ver;
    return 0;
  }
  else 
    return -1;
}

This will avoid a copy should the size of ver be 1, in which case you don't want to copy the string into version.