Skip to main content
2 of 3
solution
200_success
  • 144.2k
  • 22
  • 188
  • 473

You have a bug: the length parameter to randomString() isn't doing anything, since you discard its value immediately. You should separate your concerns into two functions: one "pure" function that takes a length parameter and generates a random string of that length, and another function that does all the DOM interaction and uses the first function.

There are three repeated implementations inside randomString(), which is very repetitive. Don't repeat yourself. I repeat, don't repeat yourself. ☺ Instead, find a way to collapse all three cases into one.

(function () {
  var $length, $result, $new, $chars;
  
  $(document).ready(function () {
    $length = $('#pw-length');
    $result = $('#pw-result');
    $new = $('#get-new-pw');
    $chars = $('#pw-chars');
    
    // display the result on page load
    getRandomString();
    // generate new password
    $new.click(function () {
      getRandomString();
      return false;
    });
    
    // allow only numbers in the length-input-field
    $length.on('keyup focusout', function () {
      $length.val($length.val().replace(/[^0-9]/g, ''));
    });
  });
  
  // Crypto provider stub for use as a fallback
  function dumbCrypto(randomIntLimit) {
    return {
      getRandomValues: function(values) {
        for (var i = 0; i < array.length; i++) {
          values[i] = Math.floor(Math.random() * randomIntLimit);
        }
      }
    };
  }
  
  function randomString(length, charset, allowRandomSourceFallback) {
    // Prefer to use a built-in CSPRNG.  Neither window.crypto nor
    // window.msCrypto is standard.
    var crypto = (window.crypto && window.crypto.getRandomValues) ? window.crypto :
                 (window.msCrypto && window.msCrypto.getRandomValues) ? window.msCrypto :
                 allowRandomSourceFallback ? dumbCrypto(charset.length) : null;
    if (crypto === null) {
      throw "No secure crypto provider available";
    }
    
    // window.crypto and window.msCrypto require a typed array (which is
    // surely supported if window.crypto or window.msCrypto is defined).
    var values = Uint32Array ? new Uint32Array(length) : new Array(length);
    
    crypto.getRandomValues(values);
    var result = "";
    for (var i = 0; i < length; i++) {
      result += charset[values[i] % charset.length];
    }
    return result;
  }
  
  // Produce and display the result
  function getRandomString() {
    $result.text(randomString($length.val(), $chars.val()));
  }
}());
200_success
  • 144.2k
  • 22
  • 188
  • 473