Skip to main content
solution
Source Link
200_success
  • 144.2k
  • 22
  • 188
  • 473

The result should be displayed using jQuery.text() instead of jQuery.html().

The result should be displayed using jQuery.text() instead of jQuery.html().

solution
Source Link
200_success
  • 144.2k
  • 22
  • 188
  • 473

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

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()));
  }
}());
Source Link
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.