2
\$\begingroup\$
  public static GeneratePassword(minPassLength) {
    let small = "a b c d e f g h i j k l m n o p q r s t u v w x y z".split(' ');
    let big = "A B C D E F G H I J K L M N O P Q R S T U V W X Y Z".split(' ');
    let numbers = "0 1 2 3 4 5 6 7 8 9".split(' ');
    let special = "! \" # $ % & ( ) * + - . : ; < = > ? @ [ \ ] _ { | }".split(' ');

    var pass = "";
    for (let i = 0; i < minPassLength; i++) {
      pass += small[this.randomIntFromInterval(0, small.length - 1)];
    }
    for (let i = 0; i < minPassLength / 4; i++) {
      pass += big[this.randomIntFromInterval(0, big.length - 1)];
    }
    for (let i = 0; i < minPassLength / 4; i++) {
      pass += numbers[this.randomIntFromInterval(0, numbers.length - 1)];
    }
    for (let i = 0; i < minPassLength / 4; i++) {
      pass += special[this.randomIntFromInterval(0, special.length - 1)];
    }
    pass = pass.split('').sort(function () { return 0.5 - Math.random() }).join('');
    return pass;
 }

  private static randomIntFromInterval(min, max) // min and max included
  {
    return Math.floor(Math.random() * (max - min + 1) + min);
  }

A simple function to generate a new password with at least X (this is function parameter) chars including 1 special, 1 number and 1 uppercase.

Please a code review :-)

\$\endgroup\$
0
\$\begingroup\$

Not sure about typescript, but I think you don't need 4 for loops. Not tested but here is what i came up with on the go:

public static GeneratePassword(minPassLength) {
  let small = "a b c d e f g h i j k l m n o p q r s t u v w x y z".split(' ');
  let big = "A B C D E F G H I J K L M N O P Q R S T U V W X Y Z".split(' ');
  let numbers = "0 1 2 3 4 5 6 7 8 9".split(' ');
  let special = "! \" # $ % & ( ) * + - . : ; < = > ? @ [ \ ] _ { | }".split(' ');

  var pass = "";

  if (minPassLength % 2 == 1){
    minPassLength++;
  }
  for (let i = 0; i < minPassLength/4; i++) {
    appendPass (pass, small[this.randomIntFromInterval(0, small.length - 1)]);
    appendPass (pass, big[this.randomIntFromInterval(0, big.length - 1)]);
    appendPass (pass, numbers[this.randomIntFromInterval(0, numbers.length - 1)]);
    appendPass (pass, special[this.randomIntFromInterval(0, special.length - 1)]);
  }

  pass = pass.split('').sort(function () { return 0.5 - Math.random() }).join('');


  return pass;
}

private static randomIntFromInterval(min, max) // min and max included
{
  return Math.floor(Math.random() * (max - min + 1) + min);
}

private static appendPass(passwrd, interval)
{
  passwrd += interval;
}
\$\endgroup\$
  • \$\begingroup\$ You're welcome.. please mark it as answered if it helped \$\endgroup\$ – DrDev Nov 14 '18 at 9:23
2
\$\begingroup\$

You've four for-loops that are doing the same with different input. Make a function for this logic and call it with the different arrays.

I don't know how typescript handles it and how often you call this function, but it could be better to define your characters outside of this because generating them every time you call this function is not necessary.

Besides, you don't need to define your alphabet twice. You can use toUpperCase/toLowerCase.

And last: From an outsiders perspective, I have no idea what this code is for:

 return 0.5 - Math.random()

You should make a function for it or at the very least, write a comment for it.

\$\endgroup\$
  • \$\begingroup\$ The 0.5 - Math.random() is a basic (not very good) way to achieve string shuffle \$\endgroup\$ – Peter B Nov 13 '18 at 16:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.