2
\$\begingroup\$

let generatorButton = document.getElementById("generator");

generatorButton.addEventListener("click", () => {
    let passwordLength = 16;
    let characters = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789~!@#$%^&*()_";
    let password = "";

    for (i = 0;i < passwordLength;i++){
        password = password + characters.charAt(Math.floor(Math.random() * Math.floor(characters.length - 1)));
    }

    document.getElementById("textField").value = password;
});
:root {
    --gray: #CBC4C4;
    --blue: #029DF1;
    --white: #FFF;
}

* {
    font-family: Roboto, sas-serif;
    margin: 0;
    padding: 0;
    margin-top: 15px;
}

body {
    background: var(--blue);
}

.container {
    width: 300px;
    margin: 0 auto;
    display: flex;
    flex-direction: column;
    justify-content: center;
    align-items: center;
    background: var(--gray);
    font-size: large;
    border-radius: 30px;
}

button {
    background: var(--blue);
    color: var(--white);
    padding: 20px 20px;
    margin: 10px 5px;
    font-size: large;
    cursor: pointer;
}

button:focus {
    border: 2px solid var(--blue);
    outline: none;
}

input {
    font-size: large;
    border: 3px solid var(--blue);
    outline: none;
    margin-bottom: 15px;
}

input::placeholder {
    opacity: 0.6;
}
<!DOCTYPE html>
<html lang="en">
<head>
    <link rel="stylesheet" href="style.css">
    <link href="https://fonts.googleapis.com/css?family=Roboto&display=swap" rel="stylesheet">
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <meta http-equiv="X-UA-Compatible" content="ie=edge">
    <title>Password Generator</title>
</head>
<body>
    <div class="container">
        <h2>Password Generator</h2>
        <button id="generator">Generate!</button>
        <input type="text" id="textField" placeholder="Your password goes here...">
    </div>
    <script src="script.js"></script>
</body>
</html>

I could add a few things onto this, like having a way to copy onto the clipboard and such. But really what I want to know is, how can I make this code shorter and more efficient? Specifically, my CSS is long. Maybe I should start using a CSS framework? Let me know.

\$\endgroup\$
  • \$\begingroup\$ Although discussing a C version, you should read some of the comments here concerning the method and cryptographic security. They are relevant and easily translate into javascript. \$\endgroup\$ – Peter Jennings Oct 4 '19 at 0:37
3
\$\begingroup\$

JavaScript

You don't have to list down all characters. All printable characters in the ASCII range are found in 33-126. What you could do is generate a random number within this range, and convert them into characters using String.fromCharCode().

Also, it's always a good practice to separate business logic from view logic. You will want to be able to generate random passwords regardless of what your UI implementation is. In this case, I recommend putting your logic into its own function, and have the event handler call it.

Store elements you get from document.getElementById() and similar APIs to a variable, and use that variable to operate on the element. DOM operations are slow, and you will want to reduce it if possible. This example is small, and the impact is negligible, but it's a good thing to keep in mind.

CSS

For your CSS, I discourage the use of element selectors, unless you're writing a globally applied style. The same goes for the * selector. The problem with using these selectors is that they apply styles globally and will affect not just your code, but code from other developers in your team.

Elements have default styles provided by the browser, and some developers expect this when styling their CSS. If I build a perfectly fine component, but your styles globally turn the font-size to 10px, that's not a nice developer experience.

Use classes instead to target specific elements. I recommend you learn the BEM naming scheme. This allows you to namespace your CSS so that it's contained only to the component/widget you're working on. In this case, your password generator.

Never do outline: none;. This is bad for accessibility and keyboard navigation. Also, do not remove visual cues for interactive elements as this makes your app look non-responsive. For example, the native styling of a button will make it look pressed when you click. If you replace the border color, you lose this. So you should provide an :active styling and style it to give feedback to the user that a press did happen.

Also not a fan of font: large as this may be different on different browsers. Use a fixed value using the rem unit. rems are based on the font-size of the <html> element. By default, <html> is 16px which means 16px * 1rem = 16px. 1.5rem is 16px * 1.5rem = 24px, and so on. The neat thing about rem is if the user has a bumped up or bumped down font-size, the your font scales with it as well.

const getRandomCharCode = (start, end) =>
  Math.floor(start + (Math.random() * ((end + 1) - start)))

// Generate an array that's length long, and fill it with random characters
// from 33 to 126 of the ascii range.
const generatePassword = length =>
  Array(length).fill()
  .map(_ => String.fromCharCode(getRandomCharCode(33, 126)))
  .join('')

const generatorButton = document.getElementById("generator")
const generatorTextField = document.getElementById("textField")

generatorButton.addEventListener("click", () => {
  generatorTextField.value = generatePassword(16)
})
:root {
  --gray: #CBC4C4;
  --blue: #029DF1;
  --blue: #029DF1;
  --dark-blue: #007DD1;
  --white: #FFF;
}

body {
  background: var(--blue);
  font-family: sans-serif;
}

.generator {
  width: 300px;
  margin: 0 auto;
  padding: 15px;
  background: var(--gray);
  border-radius: 30px;
  display: flex;
  flex-direction: column;
  justify-content: center;
  align-items: center;
}

.generator__button {
  background: var(--blue);
  color: var(--white);
  padding: 15px;
  margin: 0 0 15px;
  font-size: 1.25rem;
  cursor: pointer;
  border: 2px solid var(--blue);
}

.generator__button:active {
  background: var(--dark-blue);
}

.generator__field {
  margin: 0 0 15px;
  font-size: 1.25rem;
  border: 3px solid var(--blue);
}

.generator__field::placeholder {
  opacity: 0.6;
}
<div class="generator">
  <h2>Password Generator</h2>
  <button class="generator__button" id="generator">Generate!</button>
  <input class="generator__field" id="textField" type="text" placeholder="Your password goes here...">
</div>

| improve this answer | |
\$\endgroup\$

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.