0

I've begun learning about thread manipulation, and have begun with a simple program meant to generate random capital letters. The letters are randomly generated and added to a char array through the producer, and any added are outputted as lowercase. The consumer simply outputs the regular capital letter from the char array. So far, I have the following:

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

std::mutex mtx;
std::condition_variable cv;

int count = 0, buff_size = 0;
char* buff;

int random_int(int lower_bound) {
    std::random_device seed;
    std::mt19937 generator(seed());
    std::uniform_int_distribution<int> dist(lower_bound, std::nextafter(26, DBL_MAX));

    return dist(generator);
}

char random_char(int lower_bound) {
    return 'A' + (random_int(lower_bound) % 26);
}

/* Consumer

Reads characters from the buffer and prints them.

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

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

    /*
    job + 1 = Running
    job = Ready
    */
    for (int i = 0; i < buff_size; i++) {
        std::cout << buff[i] << std::endl;
    }

    count--;
}

/* Producer

Randomly generates letters at (pos > buff_size & pos <= 26),
inserts them at the next available position in the buffer,
and then prints out the lowercase form of the inputted letter.

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

    for (int i = 0; i < buff_size; i++) {
        buff[i] = random_char(buff_size);
        std::cout << tolower(buff[i]) << std::endl;
    }

    count++;
    cv.notify_one();
}

int main() {
    int buf_size = 0;

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

    if (buf_size > 0 && buf_size <= 26) {
        // set the buffer size
        buff_size = buf_size;
        buff = new char[buff_size];
    }
    else {
        // rage quit
        exit(1);
    }

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

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

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

    // free the allocated memory
    delete[] buff;
}

My output however is a mixture of capital letters and random numbers. What's wrong? This is my first time experimenting, so be gentle. :)

Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
T145
  • 1,415
  • 1
  • 13
  • 33
  • 2
    You've tagged C++11 in this post, and I also see `buff = new char[buff_size]` in your code. I would recommend using `std::vector` instead of an array, or at least using a smart pointer like `std::unique_ptr` or `std::shared_ptr` to manage dynamically allocated things - they will help ensure you don't forget to delete them. – Steve Sep 17 '15 at 18:47
  • `std::tolower` returns `int`. – zch Sep 17 '15 at 19:24
  • @Steve Thanks for recommendation! I was going to use a data structure like those, but since this is my first attempt I just wanted to visualize everything together. Not sure if this is more of an SO or CR question really. Would you be willing to post an answer with some examples of those data structures? The last two are new to me. – T145 Sep 17 '15 at 20:32
  • [Smart Pointer on Wikipedia](https://en.wikipedia.org/wiki/Smart_pointer). Also see [Google](http://www.google.com). – Steve Sep 17 '15 at 23:43

1 Answers1

3

output however is a mixture of capital letters and random numbers.

See zch comment. I think he means:

Instead of:

std::cout << tolower(buff[i]) << std::endl;

try

std::cout << char(tolower(buff[i])) << std::endl;

or

std::cout << (char)tolower(buff[i]) << std::endl;

Because C++ tag, you maybe should use static_cast

std::cout << static_cast<char>(tolower(buff[i])) << std::endl;       
2785528
  • 5,438
  • 2
  • 18
  • 20
  • As this is tagged C++, `static_cast` should be preferred over C-style casts. – Felix Glas Sep 17 '15 at 21:45
  • It works great, thanks! For the future posterity: SO link about static_cast. Was there anything you saw that was wrong or could be improved about my actual producer-cosumer implementation? – T145 Sep 17 '15 at 23:07
  • @T145 There is another Stack Exchange site called [Code Review](http://codereview.stackexchange.com/) that would be a perfect place to ask for feedback about working code. – Steve Sep 17 '15 at 23:48