Search code examples
c++11unique-ptrmemberstdbind

How to use std::bind properly with std::unique_ptr


I am trying to std::bind class functions in combination of std::unique_ptr and I have a lot of trouble getting it to work

First I have two classes

class simpleClass{
    public:
        simpleClass(int x){
            this->simpleNumber = x;
        }

        int simpleNumber;

        simpleClass(const simpleClass &toBeClone){
            this->simpleNumber = toBeClone.simpleNumber;
        }
        simpleClass clone(){
            simpleClass *cloned = new simpleClass(*this);
            return *cloned;
        }
};
class   className{
    public:
        className(doube input){
            this->someVariable = input;
        }

        void someFunction(std::vector<double> x, double c, std::unique_ptr<simpleClass> &inputClass, std::vector<double> &output){
            std::vector<double> tempOutput;
            for(int i = 0; i<x.size(); i++){
                tempOutput.push_back(x[i] + c * this->someVariable + inputClass->simpleNumber);
            }
            output = tempOutput;
        }

        double someVariable;

        className(const className &toBeClone){
            this->someVariable = toBeClone.someVariable;
        }

        className clone(){
            className *cloned = new className(*this);
            return *cloned;
        }
};

They are both some standard class, but I also implement a clone function to duplicate an initialized class. While cloning, I need to ensure that the original class and the cloned class points to different address. So I use std::unique_ptr to ensure this.

The is the main function, which also shows how I "clone"

int main(){
    className testSubject(5);
    std::vector<std::unique_ptr<className>> lotsOfTestSubject;

    simpleClass easyClass(1);
    std::vector<std::unique_ptr<simpleClass>> manyEasyClass;


    for(int i = 0; i<10; i++){
        std::unique_ptr<className> tempClass(new className(testSubject.clone()))
        lotsOfTestSubject.push_back(std::move(tempClass));    

        std::unique_ptr<simpleClass> tempEasyClass(new simpleClass(easyClass.clone()))
        manyEasyClass.push_back(std::move(tempEasyClass));    
    }

    std::vector<std::vector<<double>> X; //already loaded with numbers
    double C = 2;
    std::vector<std::vector<<double>> OUT;

    for(int i = 0; i<10; i++){
        std::vector<double> tempOUT;
        lotsOfTestSubject[i]->someFunction(X[i], C, manyEasyClass[i], tempOUT);
        OUT.push_back(tempOUT);

        //Here if I want to bind
        /*
           std::bind(&className::someFunction, lotsOfTestSubject[i], X[i], C, manyEasyClass[i], tempOUT);
        */
    }
    return 0;
}

The reason why I "clone" is because both simpleClass and className takes a lot of time for construction in my implementation, and I need a lot of them. And Since many of them will be initialized with the same parameters, I figured this is the easiest way to do so.

The code above works, but I am trying to improve the speed of the loop. The following line is where most of the computation takes place.

lotsOfTestSubject[i]->someFunction(X[i], C, manyEasyClass[i], tempOUT);

So I am attempting to use threads to delegate the work , and as far as I know, I need to std::bind first. So I tried

std::bind(&className::someFunction, lotsOfTestSubject[i], X[i], C, manyEasyClass[i], tempOUT);

But the compiler prints error like this

/usr/include/c++/5/tuple|206|  recursively required from ‘constexpr std::_Tuple_impl<_Idx, _Head, _Tail ...>::_Tuple_impl(const _Head&, const _Tail& ...) [with long unsigned int _Idx = 1ul; _Head = std::vector<double>; _Tail = {double, std::unique_ptr<simpleClass, std::default_delete<simpleClass> >, std::unique_ptr<simpleClass, std::default_delete<simpleClass> >, std::vector<double>}]’|
/usr/include/c++/5/tuple|108|error: use of deleted function ‘std::unique_ptr<_Tp, _Dp>::unique_ptr(const std::unique_ptr<_Tp, _Dp>&) [with _Tp = className; _Dp = std::default_delete<className>]’|

I have no idea what this means as I just started self teaching c++. Any feedback and guidance is much appreciated. I am using c++11 and g++ (Ubuntu 5.4.0-6ubuntu1~16.04.11) 5.4.0 20160609

Update

Thanks @rafix07, tried your solution and it works fine. but then I tried to do

auto theBinded = std::bind(&className::someFunction, &lotsOfTestSubject[i], 
          X[i], C, std::ref(manyEasyClass[i]), tempOUT);
std::thread testThread(theBinded);

and eventually want to testThread.join() But the compiler says

error: pointer to member type ‘void (className::)(std::vector<double>, double, std::unique_ptr<simpleClass>&, std::vector<double>&)’ incompatible with object type ‘std::unique_ptr<className>’|

@kmdreko Thanks for you point out! I haven't notice memory leak yet, but I will fix it. Do I just use this?

std::unique_ptr<className> tempClass = new className(testSubject);

Solution

  • EDIT

    If you want to call someFunction on instance stored in lotsOfTestSubject you need to pass pointer to className object on which this method will be called, so the line below

    std::bind(&className::someFunction, lotsOfTestSubject[i]
    

    should be replaced by:

    auto theBinded = std::bind(&className::someFunction, lotsOfTestSubject[i].get(), 
                                                                              ^^^          
    

    Second change is to use std::ref to pass original instance of unique_ptr of manyEasyClass instead of its copy. std::bind always copies or moved its arguments (see reference), but unique_ptr is non-copyable, that is why compilation failed.

    So fixed line looks:

    auto theBinded = std::bind(&className::someFunction, lotsOfTestSubject[i].get(), 
          X[i], C, std::ref(manyEasyClass[i]), std::ref(tempOUT));
    

    tempOUT also must be passed by std::ref because you want to modify this vector by call operator() on functor created by bind.

    LIVE DEMO