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 :-)
2 Answers 2
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;
}
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.
-
\$\begingroup\$ The
0.5 - Math.random()
is a basic (not very good) way to achieve string shuffle \$\endgroup\$Peter B– Peter B2018年11月13日 16:41:34 +00:00Commented Nov 13, 2018 at 16:41