Producer-Consumer in C++ - follow-up

Question

To expand from the changes supplied by the answer of my previous question:

#include <condition_variable>
#include <iostream>
#include <random>
#include <mutex>
#include <thread>
#include <vector>

// global variables
std::condition_variable cv;
std::mutex mtx;
std::vector<char> data;

int count = 0, buff_size = 0;

char random_char() {
    thread_local std::random_device seed;
    thread_local std::mt19937 generator(seed());
    thread_local std::uniform_int_distribution<int> dist('A', 'Z');

    return static_cast<char>(dist(generator));
}

/* Consumer

Prints out the contents of the shared buffer.

*/
void consume() {
    std::unique_lock<std::mutex> lck(mtx);

    while (count == 0) {
        cv.wait(lck);
    }

    for (const auto& it : data) {
        std::cout << it << std::endl;
    }

    count--;
}

/* Producer

Randomly generates capital letters in the range of A to Z,
prints out those letters in lowercase, and then
inserts them into the shared buffer.

*/
void produce() {
    std::unique_lock<std::mutex> lck(mtx);

    char c = random_char();
    std::cout << " " << static_cast<char>(tolower(c)) << std::endl;
    data.push_back(c);

    count++;
    cv.notify_one();
}

int main() {
    std::cout << "The Producer-Consumer Problem (in C++11!)" << std::endl << "Enter the buffer size: ";
    std::cin >> buff_size;

    // keep the buffer in-range of the alphabet
    if (buff_size < 0) {
        buff_size = 0;
    }
    else if (buff_size > 26) {
        buff_size = 26;
    }

    std::thread production[26], processed[26];

    // initialize the arrays
    for (int order = 0; order < buff_size; order++) {
        production[order] = std::thread(produce);
        processed[order] = std::thread(consume);
    }

    // join the threads to the main threads
    for (int order = 0; order < buff_size; order++) {
        processed[order].join();
        production[order].join();
    }

    std::cout << "Succeeded with a shared buffer of " << data.size() << " letters!";
}

Answer 1

Global variables in the best of cases are questionable, but in this case especially you have buff_size as a global variable. buff_size is only used in main() so should be a local variable there.

You can also take advantage of the fact that std::condition_variable::wait() has another overload that takes a predicate to replace:

while (count == 0) {
    cv.wait(lck);
}

with:

cv.wait(lck, []{ return count > 0; });

Although, did you really want every consumer to print out every letter every time? If buff_size were 10, you'd end up logging 55 letters. Was that intentional? If it wasn't, you could drop count entirely, switch data to be a std::queue and have your consumer be:

std::unique_lock<std::mutex> lck(mtx);
cv.wait(lck, []{return !data.empty(); });
std::cout << data.front() << std::endl;
data.pop();

On the producer front, did you really mean notify_one() and not notify_all()? Let all the consumers fight for the lock!

Lastly, this:

if (buff_size < 0) {
    buff_size = 0;
}
else if (buff_size > 26) {
    buff_size = 26;
}

is a clamp. It might be worth writing something like:

template <typename T>
T clamp(T const& val, T const& lo, T const& hi)
{
    return std::min(std::max(val, lo), hi);
}

buff_size = clamp(buff_size, 0, 26);