Why does STL Algorithm for_each call the destructor of my functor twice?

I was experimenting with the STL algorithms and more specific with the for_each function. I tried a simple use case for concatenating a vector of strings. Note that this is probably not a good and/or efficient code. Take a look at the boost::algorithm::join function, if you really want to concatenate a vector of strings.

#include <iostream>
#include <string>
#include <vector>
#include <algorithm>
#include开发者_高级运维 "concatenator.h"

using namespace std;

int main(int argc, char **argv) {
     vector<string> list;
     list.push_back("hello");
     list.push_back("world");
     list.push_back("!!!");
     Concatenator concatenator;
     for_each(list.begin(), list.end(), concatenator);
     cout << "result = " << concatenator.getResult() << endl;
}

The concatenator class is implemented as a regular functor.

concatenator.h:

#include <string>

class Concatenator {
    public:
        Concatenator();

        virtual ~Concatenator();

        void operator()(const std::string s);

        std::string getResult();
    private:
        std::string fResult;
};

concatenator.cpp:

#include "concatenator.h"
#include <iostream>

Concatenator::Concatenator() :
        fResult("") {
    }

Concatenator::~Concatenator(){
    std::cout << "concatenator destructor called " << std::endl;
}

void Concatenator::operator()(const std::string s) {
    std::cout << "concat " << s << " to " << this->fResult << std::endl;
    this->fResult += " " + s;
}

std::string Concatenator::getResult() {
    return this->fResult;
}

If you compile and run this program, you get the following output:

concat hello to

concat world to hello

concat !!! to hello world

concatenator destructor called

concatenator destructor called

result =

concatenator destructor called

Can anybody explain why I can't extract the right result from the functor and why the destructor is called so many times.

---

std::for_each takes the functor object by value, not by reference. It then returns it by value. In other words, your original functor object never gets modified. So you need to do:

concatenator = for_each(list.begin(), list.end(), concatenator);

Incidentally, pass-by-value necessarily creates a copy of the object, hence the extra destructor calls.

---

The function object is passed to for_each by value and returned by for_each by value, so three instances of Concatenator get created:

1. You create one instance using Concatenator concatenator;
2. You pass this object to for_each and it is copied because for_each takes it by value
3. for_each returns the functor by value, causing another copy to be created

Each of these three objects is destroyed, hence the destructor gets called three times.

---

When you implement a destructor, it's likely that you also need to implement a copy constructor and copy assignment operator. This is known as the rule of three.

Once you correctly implement those two methods, then you'll see that it's not calling the destructor twice, but instead it's making a copy and destructing each of the copies.

---

Other answers already explained to you that the problem in your case is functor object being passed to for_each and returned from for_each by value.

However, while it is true that the declaration of for_each is responsible for this behavior, the final word is said by the template argument deduction mechanism of C++. In accordance with the rules of the language, in this call

for_each(list.begin(), list.end(), concatenator);

the second argument of for_each template is deduced as Concatenator and not as Concatenator &, resulting in pass-by-value semantics.

You can override the deduction by specifying template arguments explicitly, insisting on the reference type for the second template argument

for_each<vector<string>::iterator, Concatenator &>(ist.begin(), list.end(),
    concatenator);

This will completely eliminate the copying and replace the pass-by_value semantics with pass-by-reference semantics (also covering the return value of for_each). It doesn't look as elegant, especially because functor type happens to be the second template argument, but it is a workaround.

---

for_each takes the functor by value and returns a copy of it as the return value, so it's not your concatenator that is modified, but the one local to the for_each function that is then returned. You should change your code to something like that:

 Concatenator concatenator;
 concatenator = for_each(list.begin(), list.end(), concatenator);

In concatenator now you'll have your modified functor.

About the destructor calls: they begin when for_each returns; the first one is the one of the parameter of for_each, the second one is the one of its copy returned by for_each (which is discarded), the third one is the one of the original concatenator object, which is destructed when the program ends.