0

I am trying to create a random password generator. When I call the function multiple times it returns the same character value. QQpQ;7Q7pQ;p

I have tried adding srand(time(0)); or srand((unsigned int) time(NULL)); in my main function.

Looking at other stack over flow posts I see that srand helps create different characters each time I run the program. However it still returns multiples of the same character.

How can I change srand() seed during run time to get different random numbers?

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

using namespace std;

string randomGenerator(int numberOfCharacters);
char randomCapitalCharacter();
char randomLowerCharacter();
char randomSpecialCharacter();
char randomInt();

int main(){
    srand((unsigned int) time(NULL));
    int passwordLength = 12;
    string password = randomGenerator(passwordLength);
    cout << password << endl;
    return 0;
}

string randomGenerator(int numberOfCharacters){
    char randomCharacterTypes[4] = {randomLowerCharacter(),randomInt(), randomCapitalCharacter(), randomSpecialCharacter()};
    std::string password;

    while (numberOfCharacters > 0){
        int random = rand()%4;
        password += randomCharacterTypes[random];
        numberOfCharacters--;
    }
    return password;
}
char randomInt(){
    std::string numberAsString = to_string(std::rand()% 10);
    char randomNumberChar = numberAsString.at(0);
    return randomNumberChar;
}
char randomLowerCharacter(){
    return 97 + rand()%26; //97 = a
}
char randomCapitalCharacter(){
    return 65 + rand()%26; //65 = A
}
char randomSpecialCharacter(){
    /** Special characters are split by numbers so we pick a random from one of the 2 groups.*/
    return (rand()%2) ? char(33 + rand()%14) : char(58 + rand()%8); 
}
Ted Lyngmo
  • 93,841
  • 5
  • 60
  • 108
  • `randomCharacterTypes[4]` only contains 4 different characters, did you mean to create an array of function pointers? – Alan Birtles Apr 13 '21 at 06:17
  • When I run your code, the first time it prints `RReR>>eReR6e` and the second time it prints `g>g>7gTgg>>T` Is it because you are running it more than once in the same second? That would give the same values because time(NULL) returns an integer number seconds - so if you run it more than once in the same second time(NULL) would return the same value. – Jerry Jeremiah Apr 13 '21 at 06:19
  • 4
    Since you are using c++11, you should stop using srand in favour of the new random number generators: https://stackoverflow.com/a/66850447/1294207 – Fantastic Mr Fox Apr 13 '21 at 06:20
  • @AlanBirtles Oh I see yes I am trying to use function pointers. I see that if I just call the randomLowerCharacter(); that it does return a random character value. I will look up how to implement function pointers. Thank you!!! – mfischercodes Apr 13 '21 at 06:20
  • You'd probably be better off using a switch statement after `int random = rand()%4;` to call the right function and get a character. Right now you're only calling each function once and storing the result in `randomCharacterTypes`. It looks like maybe you're hoping that you're calling the function each time but you're not and a switch may make more sense than trying to explain how function pointers work. – Retired Ninja Apr 13 '21 at 06:21
  • 2
    You can avoid the need for comments like `//97 = a` if you write `'a'` instead of `97` in the code (and similar for other characters).. – molbdnilo Apr 13 '21 at 06:25
  • @RetiredNinja Perfect thank you that works beautifully. I'll still look into function pointers as I think it would look cleaner. But for now the switch statement has my program working as intended. Thank you!! – mfischercodes Apr 13 '21 at 06:26
  • Don't put answers in your question. Write a proper answer instead. I rolled back to your original question. – Ted Lyngmo Apr 13 '21 at 06:47

2 Answers2

2

Thank you to @RetiredNinja for pointing out that the problem wasn't with srand() but rather with the improper calling of functions in my array. Using a switch statement to call the functions properly fixed the issue.

//improper calling of functions in an array. Resolved with switch statement below
//char randomCharacterTypes[4] = {randomLowerCharacter(),randomInt(), randomCapitalCharacter(), randomSpecialCharacter()};

while (numberOfCharacters > 0){
    int random = rand()%4;
    switch(random){
    case 0:
        password += randomLowerCharacter();
        break;
    case 1:
        password += randomInt();
        break;
    case 2:
        password += randomCapitalCharacter();
        break;
    case 3:
        password += randomSpecialCharacter();
        break;
    }
    numberOfCharacters--;
}
CiaPan
  • 9,381
  • 2
  • 21
  • 35
  • You may simplify the loop a bit by dropping the decrementation at the end of the loop body and incorporating it in the conditition instead: `while(--numberOfCharacters >= 0) {...}` – CiaPan Apr 13 '21 at 07:13
2

randomCharacterTypes is an array of 4 characters, for each character class you'll always get the same character. If you want to generate a new character each time you could use function pointers instead:

string randomGenerator(int numberOfCharacters){
    char (*randomCharacterTypes[])() = {randomLowerCharacter,randomInt, randomCapitalCharacter, randomSpecialCharacter};
    std::string password;

    while (numberOfCharacters > 0){
        int random = rand()%4;
        password += randomCharacterTypes[random]();
        numberOfCharacters--;
    }
    return password;
}

randomInt can be simplified to return a character between '0' and '9' the same as your other functions:

char randomInt(){
    return '0' + rand() % 10;
}

Your other functions are more readable if you use character rather than numbers which you then have to add a comment to remind you which character they represent:

char randomLowerCharacter(){
    return 'a' + rand()%26;
}
char randomCapitalCharacter(){
    return 'A' + rand()%26;
}
char randomSpecialCharacter(){
    /** Special characters are split by numbers so we pick a random from one of the 2 groups.*/
    return (rand()%2) ? char('!' + rand()%14) : char(':' + rand()%8); 
}

note that rand() % x is not a good random number generator and you should use the standard library random functions instead (especially if you are using these passwords for anything important).

Your code could possibly be further simplified using strings to contain your character classes:

#include <string>
#include <string_view>
#include <iostream>
#include <random>
#include <array>

class PasswordGenerator
{
public:
    PasswordGenerator()
    {
        for (size_t i = 0; i < characterTypes.size(); i++)
        {
            characterTypeDistributions[i] = std::uniform_int_distribution<int>(0, characterTypes[i].size() - 1);
        }
    }

    static constexpr std::array<std::string_view, 4> characterTypes = {
            "abcdefghijklmnopqrstuvwxyz",
            "ABCDEFGHIJKLMNOPQRSTUVWXYZ",
            "0123456789",
            "!\"#$%&'()*+,-./:;<=>?@"
        };
    std::array<std::uniform_int_distribution<int>, characterTypes.size()> characterTypeDistributions;
    std::uniform_int_distribution<int> typeDistribution{0, characterTypes.size() - 1};
    std::mt19937 eng{std::random_device{}()};

    std::string generate(int numberOfCharacters){
        std::uniform_int_distribution<int> dist{0, 5};
        std::string password;
        password.reserve(numberOfCharacters);
        for (int i = 0; i < numberOfCharacters; i++)
        {
            auto type = typeDistribution(eng);
            password += characterTypes[type][characterTypeDistributions[type](eng)];
        }
        return password;
    }
};

int main(){
    int passwordLength = 12;
    PasswordGenerator gen;
    std::string password = gen.generate(passwordLength);
    std::cout << password << "\n";
    return 0;
}

Note that this code may still not meet high security standards for generating passwords.

Alan Birtles
  • 32,622
  • 4
  • 31
  • 60
  • at least 600 MT19937 outputs are needed before having much of a chance of recovering its state, which you're well below here. that said, it's probably better to just exclusively use `random_device` for the engine instead of using `mt19937` at all – Sam Mason Apr 13 '21 at 09:13
  • @SamMason depends on the quality of `random_device`... – Alan Birtles Apr 13 '21 at 09:38
  • this is indeed permitted by the standard, but I'd be intrigued to know on what real implementations this would actually result in a less predictable password. e.g. under linux, osx and windows with any mainstream compiler I'd certainly prefer not to involve `mt19937` – Sam Mason Apr 13 '21 at 09:58