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 :-)

200_success
145k22 gold badges190 silver badges478 bronze badges
asked Nov 13, 2018 at 15:04
\$\endgroup\$

2 Answers 2

1
\$\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;
}
answered Nov 13, 2018 at 16:57
\$\endgroup\$
0
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.

answered Nov 13, 2018 at 15:21
\$\endgroup\$
1
  • \$\begingroup\$ The 0.5 - Math.random() is a basic (not very good) way to achieve string shuffle \$\endgroup\$ Commented Nov 13, 2018 at 16:41

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.