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;
}
