Search code examples
javascriptmathrandompassword-generator

Why does my password generator written in JavaScript often return passwords with repeating characters?


I've written a JavaScript class with four static variables containing the different kinds of characters the final password may include. The class contains four getter functions which return a random character from these four variables. In a separate function I try to create the password. Unfortunately the final password does not seem to contain wholly random characters. Often same characters are repeated.

I had a closer look at my random-function but it seems to be fine. I hope you have an idea why the passwords end up being so similar e.g. AAAh^8 or bSS+5S

class Password{
    
    static lowerCase = "abcdefghijklmnopqrstuvwxyz";
    static upperCase = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
    static numbers   = "0123456789";
    static symbols   = "!@#$%^&*()_+~\\`|}{[]:;?><,./-=";

    length = document.getElementById("passwordLength").value;

    getKey = [this.upperCase, this.lowerCase, this.num, this.symbol]

    get upperCase(){
        return Password.upperCase[Math.floor(Math.random() * Password.upperCase.length)]
        }
    get lowerCase(){
        return Password.lowerCase[Math.floor(Math.random() * Password.lowerCase.length)]
    }
    get num(){
        return Password.numbers[Math.floor(Math.random() * Password.numbers.length)]
    }
    get symbol(){
        return Password.symbols[Math.floor(Math.random() * Password.symbols.length)]
    }
    password = ""
}

function createPassword(){
    const newPass = new Password;
     
    for (var i=0; i<newPass.length; i++){
        key = newPass.getKey[Math.floor(Math.random() * newPass.getKey.length)]
        newPass.password += key
        console.log("Random Key Run " + i + ": " + key)
    }
    console.log("Final Password: " + newPass.password)
}
createPassword()

Solution

  • As @deceze says, you're essentially pre-selecting your 4 different characters in the getKey initializer.

    You'll probably have a better time if you're less fancy with getters, e.g.

    function pick(arr) {
      return arr[Math.floor(Math.random() * arr.length)];
    }
    
    class PasswordGenerator {
      static lowerCase = "abcdefghijklmnopqrstuvwxyz";
      static upperCase = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
      static numbers = "0123456789";
      static symbols = "!@#$%^&*()_+~\\`|}{[]:;?><,./-=";
      static classes = [
        PasswordGenerator.lowerCase,
        PasswordGenerator.upperCase,
        PasswordGenerator.numbers,
        PasswordGenerator.symbols,
      ];
    
      getChar() {
        const cls = pick(PasswordGenerator.classes);
        return pick(cls);
      }
    }
    
    function createPassword() {
      const generator = new PasswordGenerator();
      let password = "";
      for (var i = 0; i < 8; i++) {
        const key = generator.getChar();
        password += key;
        console.log(`Random Key Run ${i}: ${key}`);
      }
      console.log("Final Password: " + password);
    }
    
    createPassword();
    

    I also took the liberty of moving stuff that shouldn't necessarily be Password state out of the PasswordGenerator class.