1
\$\begingroup\$

I recently saw a youtube video on printing even and odd numbers from 0 to 100 in Java. https://www.youtube.com/watch?v=eRNTx8k5cmA

Thread 1 prints even numbers and thread 2 prints odd numbers. Both the threads print the numbers till 100.

I tried to write something similar in C++. But C++ does not have synchronized keyword and the important sync variables are global. Is there anyway to eliminate the global variables, yet keep the code as flexible and at the same time not repeat the code ?

#include<cstdio>
#include <iostream>
#include <array>
#include <thread>
#include <mutex>
#include <condition_variable>

std::mutex m;
std::condition_variable cv;
bool isEvenGlob = true;

void printNum(int initialVal, int finalVal, int step, bool isEven)
{
    for (int i = initialVal; i <= finalVal; i += step)
    {
        std::unique_lock<std::mutex> lk(m);
        
        cv.wait(lk, [isEven](){
            return !(isEven ^ isEvenGlob);
        });
        
        std::cout << i << ' ';
        
        isEvenGlob = !isEvenGlob;
        
        lk.unlock();
        cv.notify_one();
    }
    
}
int main ()
{
    std::thread thEven(printNum, 0, 100, 2, true);
    std::thread thOdd(printNum, 1, 100, 2, false);
    
    thEven.join();
    thOdd.join();
}

I wrote another version without global variables, but I am still not sure if it is an acceptable answer, especially since isEven is repeated twice -

#include<cstdio>
#include <iostream>
#include <array>
#include <thread>
#include <mutex>
#include <condition_variable>

class PrintNums {
    public:
    PrintNums(std::mutex& m, std::condition_variable& cv, bool& isEvenGlob) :
        cv_(cv), m_(m), isEvenGlob_(isEvenGlob)
        {
        };
    
    void printValues(int initVal, int finalVal, int step, bool isEven)
    {
        for (int i = initVal; i <= finalVal; i += step)
        {
            std::unique_lock<std::mutex> lk(m_);
        
            cv_.wait(lk, [this, isEven](){
                return !(isEven ^ this->isEvenGlob_);
            });
        
        std::cout << i << ' ';
        
        isEvenGlob_ = !isEvenGlob_;
        
        lk.unlock();
        cv_.notify_one();  
        }
    }
    private:
    int initVal_, finalVal_, step_;
    
    std::condition_variable& cv_;
    std::mutex& m_;
    bool& isEvenGlob_;
};

int main ()
{
    std::mutex m;
    std::condition_variable cv;
    bool isEvenGlob = true;
    
    PrintNums evenObj(m, cv, isEvenGlob);
    PrintNums oddObj(m, cv, isEvenGlob);
    
    std::thread t1(&PrintNums::printValues, &evenObj, 0, 100, 2, true);
    std::thread t2(&PrintNums::printValues, &oddObj, 1, 100, 2, false);
    
    t1.join();
    t2.join();
}

Both the versions are printing the correct answer

\$\endgroup\$
3
  • \$\begingroup\$ "Could you show me how you'd take something fast and simple, and use threads to turn it into something slow and complicated?" Unless I was incredibly desperate for any job I could get, if somebody asked me to do this in an interview, I'd much more likely lecture them on what a bad idea this is than make any attempt at directly "solving" the problem. \$\endgroup\$ Oct 23 '21 at 23:25
  • \$\begingroup\$ Why did you not simply use return isEven==isEvenGlob;? \$\endgroup\$
    – jdt
    Oct 24 '21 at 11:28
  • \$\begingroup\$ Good idea @upkajdt. I don't know why I chose a round about way to check that. Thanks for pointing it out. \$\endgroup\$
    – elMuon
    Oct 27 '21 at 10:04
2
\$\begingroup\$

Answers to your questions

Is there anyway to eliminate the global variables?

Yes, you can put them on the stack of a function, like you already did in your second version, or alternatively you can make those variables static member variables of the class.

yet keep the code as flexible and at the same time not repeat the code?

I don't really see any repeated code, apart from the fact that you create and destroy two threads. You could do that in a loop, but for this simple program that's more effort that it is worth.

especially since isEven is repeated twice

Maybe you meant isEvenGlob? One is an actual bool in main(), the other is a reference to a bool in class PrintNums. I wouldn't count that as repeated.

Separate the locking from the printing

I would try to separate the details of locking as much as possible from the part that prints the numbers. Ideally, you would write something like this:

void printNum(int initialVal, int finalVal, int step, bool isEven) {
    for (int i = initialVal; i <= finalVal; i += step) {
        auto lock = getLock(isEven);
        std::cout << i << ' ';
    }
}

A possible implementation of getLock() is like so:

std::unique_lock<std::mutex> getLock(bool isEven) {
    static bool nextIsEven = true;
    static std::mutex mtx;
    static std::condition_variable cond;

    // Wait until it is our turn
    std::unique_lock<std::mutex> lock(mtx);
    cond.wait(lock, [&](){ return nextIsEven == isEven; });

    // Signal the next thread it is their turn
    nextIsEven = !isEven;
    cond.notify_one();

    // But return the lock to the caller so they can do their thing first
    return lock;
}

If you want to avoid calling notify_one() while still holding the lock, you would have to implement your own lock class that has a destructor that unlocks the mutex and then notifies the condition variable, and have getLock() return an instance of that class.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.