Question

I have a function where I have to modifiy the values of a vector. is it a good practice in C++ to return the vector?

Function 1:

vector<string> RemoveSpecialCharacters(vector<string> words)
{
    for (vector<string>::iterator it=words.begin(); it!=words.end(); )
    {
        if(CheckLength(*it) == false)
        {
            it = words.erase(it);
        }
        else{
            ++it;
        }
    }//end for

    return words;
}

Function 2:

void RemoveSpecialCharacters(vector<string> & words)
{
    for (vector<string>::iterator it=words.begin(); it!=words.end(); )
    {
        if(CheckLength(*it) == false)
        {
            it = words.erase(it);
        }
        else{
            ++it;
        }
    }//end for
}
Was it helpful?

Solution

Your two functions serve for two different purposes.

  • Function 1: works as remove_copy. It will not modify the existing container; it makes a copy and modifies that instead.

  • Function 2: works as remove. It will modify the existing container.

OTHER TIPS

It's somewhat subjective. I personally would prefer the latter, since it does not impose the cost of copying the vector onto the caller (but the caller is still free to make the copy if they so choose).

In this particular case I'd choose passing by reference, but not because it's a C++ practice or not, but because it actually makes more sense (the function by its name applies modification to the vector). There seems to be no actual need to return data from the function.

But that also depends on purpose of the function. If you always want to use it in the following way:

vec = bow.RemoveSpecialCharacters(vec);

Then absolutely first option is a go. Otherwise, the second one seems to be more appropriate. (Judging by the function name, the first one seems to me to be more appropriate).

In terms of performance, the first solution in modern C++11 world will be more-less a few assignments slower, so the performance impact would be negligible.

Go for option 2, modifying the vector passed as parameter.

Side note: some coding practices suggest to pass through pointers arguments that may be changed (just to make it clear at a first glance to the developers that the function may change the argument).

The best practice is to pass by reference the vector.

Inside the function you edit it and you don't have to return it back (which in fact you are allocating a new memory space that is not needed).

If you pass it by reference the cost is much smaller and the vector is also edited out of the scope of the function

The first function will return a copy of the vector, so it will be slower than the second. You should use the second.

Actually neither one is a good practice for C++, if by good practice to mean how it is done in C++ libraries. You basically reimplement what std::remove_copy_if or std::remove_if does, so good practice is to implement functions (or use existing ones), that work on range, not on container, either by value or by reference.

Again this depends on how you define term good practice.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top