[Solved] Returning a random string

Question

Why do you include so many unneeded header files? These should be sufficient:

#include <iostream>
#include <string>
#include <stdio.h>
#include <stdlib.h>
#include <time.h>

You should avoid using namespace std;. If you don’t want to type std::string every time especially you can use using std::string;

using std::string;
using std::cout;
using std::endl;

Now to your function.

To avoid compiler warnings I would recommend to change srand(time(NULL)); to srand(static_cast<unsigned int>(time(NULL)));

Your loop is ok, yet you usually don’t write i <= stringlength - 1 but i < stringlength

The next srand(time(NULL)); is unecessary.

The real problem is your random value: You are using rand()%7+1. rand()%7 will give give you a value of range [0,6] which would be ok, but by adding 1 the range moves to [1;7] and 7 is outside of your lettersToUse string.

To be safe you should limit the range to what is actually accessible. lettersToUse.size() is 7 in your case. As we saw before rand()%7 gives you the correct range. So going with rand()%lettersToUse.size() will always be ok as long as lettersToUse.size() is not 0.

#include <iostream>
#include <string>
#include <stdio.h>
#include <stdlib.h>
#include <time.h>

using std::string;
using std::cout;
using std::endl;


string createRandomString(int);

int main() {

    string test;
    test = createRandomString(1);
    cout << test;
    return 0;

}


string createRandomString(int stringlength) {

    srand(static_cast<unsigned int>(time(NULL)));

    string lettersToUse = "ABCDEFG";
    string newOne = "";

    for (int i = 0; i < stringlength - 1; i++) {
        newOne += lettersToUse[rand() % lettersToUse.size()];
    }
    return newOne;
}

If you attempt to modify lettersToUse or even allow a user to specify lettersToUse, you should definitaly add a check for the size of letterToUse as the program will crash when it’s empty.

If you can use C++11 I would recommend to use the C++ random number generator provided by it:

#include <iostream>
#include <string>
#include <random>

using std::string;
using std::cout;
using std::endl;


string createRandomString(int);

int main() {

    string test;
    test = createRandomString(5000);
    cout << test;
    return 0;

}


string createRandomString(int stringlength) {
    string lettersToUse = "ABCDEFG";
    string newOne = "";

    std::default_random_engine generator;
    std::uniform_int_distribution<int> distribution(0, lettersToUse.size()-1); //Range from [0-6], when size is 7: 0-6 = 7 elements

    for (int i = 0; i < stringlength - 1; i++) {
        newOne += lettersToUse[distribution(generator)];
    }
    return newOne;
}

As recommended by caps here is the version using std::generate_n:

#include <iostream>
#include <string>
#include <random>
#include <algorithm>
#include <iterator>

using std::string;
using std::cout;
using std::endl;


string createRandomString(int);

int main() {

    string test;
    test = createRandomString(1);
    cout << test;
    return 0;

}


string createRandomString(int stringlength) {
    string lettersToUse = "ABCDEFG";

    std::default_random_engine generator;
    std::uniform_int_distribution<int> distribution(0, lettersToUse.size() - 1); //Range from [0-6], when size is 7: 0-6 = 7 elements

    string newOne;
    newOne.reserve(stringlength);
    std::generate_n(std::back_inserter(newOne), newOne.capacity(), [&lettersToUse, &distribution, &generator]() { return lettersToUse[distribution(generator)]; });

    return newOne;
}

Accepted Answer