0

Im trying to write a simple key generator for XOR cipher. All chars in the key must be unique, no 2 same symbols. So far i tried this:

void genKey (int Size) {
    srand(time(NULL));
    char Alphabet [63] = "1234567890qwertyuiooasdfghjklzxcvbnmqwertyuiopasdfghjklzxcvbnm";
    int aLenght = strlen(Alphabet);
    int kSize = Size;
    char Key [kSize];
    bool isInArray = false;
    for (int i = 0 ; i< kSize; i++)
    {
                int Pos = rand () % aLenght;
                char randomTemp = Alphabet[Pos];
                for (int j = 0; j < kSize; j++){
                    if (Key[j] == randomTemp)
                    {isInArray = true;}
                        else
                    {isInArray = false;}

                if (isInArray == false){Key[i] = randomTemp;}

                }
    }
    string sKey(Key);
    currentKey= sKey;

}

But it keeps generate keys with re-appearing symbols. Please help.

Stiven
  • 31
  • 3
  • 1
    The easy solution is to [`shullfe`](https://en.cppreference.com/w/cpp/algorithm/random_shuffle) the data and then taking the first N elements that you need. – NathanOliver Jan 26 '22 at 18:54
  • I'm guessing you are calling `genKey` repeatedly? It will only generate a different key when `time(NULL)` returns a different value (i.e. once per second), you should only call `srand` once at the beginning of your program or ideally use [modern random number generation](https://en.cppreference.com/w/cpp/numeric/random) – Alan Birtles Jan 26 '22 at 18:55
  • could call rand (don't forget to seed it) and use it as an index. (don't forget to remove each element you use in the array or you may use it again). I recommend std::vectors? – Jacob Glik Jan 26 '22 at 18:56
  • Reopened. The question isn't about seeing the same result string multiple times; it's about getting duplicate characters in a single result string. The error is in the inner loop, not in the use of `srand` or `rand`. – Pete Becker Jan 26 '22 at 19:20
  • Why are there duplicates in your `Alphabet`? – Thomas Matthews Jan 26 '22 at 21:17

1 Answers1

3

The problem is in the inner loop:

for (int j = 0; j < kSize; j++){
    if (Key[j] == randomTemp)
        isInArray = true;
    else
        isInArray = false;
    if (isInArray == false){Key[i] = randomTemp;}
}

The loop should exit when it sees that Key[j] is equal to randomTemp. As written, it will look at every character in Key, and only save the result of comparing the last character with randomTemp.

So:

bool isInArray = false;
for (int j = 0; j < kSize; j++){
    if (Key[j] == randomTemp) {
        isInArray = true;
        break;
    }
    if (isInArray == false)
        Key[i] = randomTemp;
}

There are a couple of other problems that will bite you once this is fixed.

First, only call srand once, at program startup (typically at the beginning of main()).

Second, that inner loop runs from 0 up to kSize, but the values that come after i in Key have not been initialized. The loop should stop at i:

for (int j = 0; j < i; j++)

Third, char Key[kSize]; is not legal C++. The size of an array has to be a compile-time constant. (Yes, some compilers allow this, as an extension)

And, finally, as one of the comments mentions, this code reinvents the wheel. Use std::shuffle to rearrange the characters in alphabet, and just use however many you need:

std::random_device rd;
std::mt19937 rng(rd());

char Alphabet [] = "1234567890abcdefghijklmnopqrstuvwxyz";
std::shuffle(std::begin(Alphabet), std::end(Alphabet), rng);
currentKey = std::string(Alphabet, Size); // copy Size characters into string
Pete Becker
  • 74,985
  • 8
  • 76
  • 165