I am a newcomer to JavaScript, and have been battling with this relatively simple program for a few days now.
The program: I have a website with a button on. When you click the button, a password is generated (from an array) and displayed for the user.
The problem: "function generate()" displays the single same generated character 10 times, instead of 10 different characters 10 times, which naturally would make a much stronger password. I want it to randomize 10 different characters into a password, not the same character 10 times. So the program works, but not as I intended.
I have created a JSfiddle containing comments for display. Note that document.write is disabled in JSfiddle, so the result is set to show in console.
https://jsfiddle.net/ohstmbym/
<!DOCTYPE html>
<html>
<head>
<title></title>
<button onclick="generate()">Button</button>
</head>
<body>
<script>
var arr = ["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"];
randomLetter = arr[Math.floor(arr.length * Math.random())];
function generate () {for (i = 0; i < 10; i++) {document.write(randomLetter); } }
</script>
</body>
</html>
Important: I would like to understand what is wrong with the code/how to fix it - I am not just looking for somebody to solve it for me with no explanation. If you want to help me solve the problem, I am very thankful, but please explain what you do. And also, please don't rewrite the code completely like in rewriting the entire program to make it work; I am looking for solutions/explanations, not somebody write the whole thing up. If it's completely wrong and need to be entirely rewritten, please simply explain in words what should've been done instead.
Still, thank you very much in advance, I appreciate any help.
(P.S. I know only lower case letters for password generation will be of weaker security, but this is only for experimentation)
-
2You are setting randomLetter only once, why would you except it to be 10 different random letters? For that you need to set it inside the forjuvian– juvian2017年05月22日 14:05:12 +00:00Commented May 22, 2017 at 14:05
-
@juvian thanks I've been trying to work my way around this, e.g. multiplying randomLetter * 10 and such, but with no luck. I will try to put it inside the for loop ASAP (I am AFK for a little while though). Thank you very much for the answerthesystem– thesystem2017年05月22日 14:09:12 +00:00Commented May 22, 2017 at 14:09
-
4relevant xkcd xkcd.com/221 :PAlessi 42– Alessi 422017年05月22日 14:09:47 +00:00Commented May 22, 2017 at 14:09
1 Answer 1
The problem is that you're not regenerating the random element, you're caching a random element once in randomLetter so it's always the same. The value of the variable will not change every time you refer to it. Put it inside the for loop to regenerate the number every iteration:
var arr = ["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"];
function generate () {
for (i = 0; i < 10; i++) {
var randomLetter = arr[Math.floor(arr.length * Math.random())];
document.write(randomLetter);
}
}
3 Comments
var declares the variable. Without var you're implicitly creating a global variable which is almost never good.