2
votes

I write a code in order to write prime number in a file, and after that I write another function in order to part the work in many threading. But my problem is the compiler write me an error

error C2664: 'void (int,int,std::ofstream &)' : cannot convert argument 3 from 
'std::basic_string,std::allocator>' to 'std::ofstream &'.

But my problem is that I don't understand what the error says and where I do a thing that is forbidden.

This is my code:

void writePrimesToFile(int begin, int end, ofstream& file)
{
    int i, j, prime = 1;

    for (i = begin; i <= end; i++) {
        for (j = begin; i <= end / 2; j++) {
            if (i % j == 0) {
                prime = 0;
            } else {
                file << i << endl;
            }
        }
    }
}

void callWritePrimesMultipleThreads(int begin, int end, string filePath, int N)
{
    double startTimer, stopTimer;

    startTimer = clock();
    thread* arr = new thread[N];
    for (int i = 0; i < N; i++) {
        int start = begin;
        int finish = N;
        arr[i] = thread(writePrimesToFile, start, finish, ref(filePath));
        start = finish;
        finish += N;
    }
    for (int i = 0; i < N; i++) {
        arr[i].join();
    }
    stopTimer = clock();
    cout << "The time that takes is: " <<
            double(stopTimer - startTimer) / CLOCKS_PER_SEC << endl;
}
1
You're passing a reference to a std::string object to your thread function, which wants a std::ofstream reference (clearly stated in the message). That will not work very well. - Some programmer dude
Oh, and you have a memory leak to. Why don't you use std::vector instead? - Some programmer dude
@Someprogrammerdude Where I have a memory leak ? Can you explain more ! - Rony Cohen
Don't do this; thread* arr = new thread[N]; do this std::vector<std::thread> arr; followed by arr.emplace_back(writePrimesToFile, start, finish... etc); - Galik
@RonyCohen Its a reference to the thread in the vector. I used && when I could have just used & out of habit (it works in more situations). So for(auto& thread: arr) basically iterates through all the threads in the vector giving you a reference to each one that you can use to join(). It is like saying for each thread in arr join() that thread. - Galik

1 Answers

4
votes

Your thread function expects third parameter as std::ofstream reference, you are trying to bind reference to std::string instead. So create std::ofstream before loop and pass it:

void callWritePrimesMultipleThreads(int begin, int end, string filePath, int N)
{
    double startTimer, stopTimer;

    std::ofstream file( filePath );
    // check that file is opened

    startTimer = clock();
    ...
         arr[i] = thread(writePrimesToFile, start, finish, ref(file));

you dynamically allocate array of thread by new[] and do not have delete[] at the end of your function hence the memory leak. It would be easier and cleaner to use std::vector<std::thread> instead:

 //thread* arr = new thread[N];
 std::vector<std::thread> arr( N ); // replace with this and you do not need to touch anything else