0
\$\begingroup\$

Just for practice purposes, I written a basic client sided login/signup system that allows you to make an account, and then log into the account from my webpage. What it does is append to an object known as database for every new account made, and when you sign in, it loops through the database and see if your credentials match any account information.

There's a couple of noticable problems with my script. One being that I believe that I didn't follow the DRY principle to the fullest. I felt like I repeated myself a couple of times, and wondering if that can be avoided. The second problem is that looping through the "database" object may not be the most efficient solution, especially since the longer the table, the longer it may take. There may be a solution that is more efficient in performance that I am unaware of.

Other than that I would just like general feedback, tips, and red flags (if any) in my code. I am trying to improve myself as a JavaScript developer, and that requires feedback of my script from my peers.

TL;DR: I am a new programmer, so sorry if this is a bad question or if I have terrible code. I am still learning.

const database = []

function usp(parent) {
  return [parent.username.value, parent.password.value]
}

window.onload = () => {
  const create = document.forms.createAcount;
  const log = document.forms.logIN;

  create.addEventListener('click', (e) => {
    if(e.target == create.submit){
      const information = usp(create);
      database.push({user : information[0], pass : information[1]});
      console.log(database)
    }
  });

  log.addEventListener('click', (e) => {
    if (e.target == log.submit) {
      const information = usp(log);
      for (let x of database) {
        if (x.user == information[0] && x.pass == information[1]) {
          alert(`You successfully logged into your account! Welcome to my website ${information[0]}`)
        }
      }
    }
  })
}
#createAcount {
  background-color: skyblue;
}
#logIN {
  background-color: lightgreen;
}
<!DOCTYPE html>
  <html>
    <head>
      <title>SignUp!</title>
      <link rel = "stylesheet" href = 'style.css'>
    </head>
    <body>
      <h1>Sign Up</h1>
      <form name = "createAcount" id = "createAcount">
        <h1>Create a Username</h1>
        <input type = 'text' placeholder="Falkyraizu"  name = 'username' maxlength=20>
        <h1>Create a password</h1>
        <input type = 'password' placeholder="password" name = 'password' maxlength=20>
        <h2>Done</h2>
        <input class = 'reset' type = 'button' value = "Reset" name = 'reset'>
        <input class = 'submit' type = 'button' value = "Submit" name = 'submit'>
      </form>
      <h1>Log In</h1>
      <form name = "logIN" id = "logIN">
        <h1>What is your userName</h1>
        <input type = 'text' placeholder="Falkyraizu" name = "username" maxlength=20>
        <h1>What is your password</h1>
        <input type = 'password' placeholder="password" name = "password" maxlength=20>
        <h2>Done</h2>
        <input class = 'reset' type = 'button' value = "Reset" name = 'reset'>
        <input class = 'submit' type = 'button' value = "Submit" name = 'submit'>
      </form>
      <script src = "script.js"></script>
    </body>
  </html>

Side Note: If you notice something else that can be fixed such as my HTML, you can also include it in your response, however I would still like my main problems fixed (read above if you missed it). Thanks for the help!

\$\endgroup\$
1
\$\begingroup\$

This code appears to be fairly clean and reduced well to me, I don't see much in the way of any DRY violations, but if you're looking for any places for tightening up your code:

If you can use the newer javascript goodies, then use array destructuring assignment and object shorthand assignment

Given

function usp(parent) {
  return [parent.username.value, parent.password.value];
}

Then

const [user, pass] = usp(create);
database.push({ user, pass });

and use '==' with caution as it isn't strictly equal, it will attempt a type coercion, i.e. '5' == 5 is true, where as '===' won't, i.e. '5' === 5 is false. As a rule, you will almost always want to use '===', only reach for '==' when necessary.

Comparison operators

const [user, pass] = usp(log);
for (const x of database) {
  if (x.user === user && x.pass === pass) {
    ...
  }
}

Looping through any data structure has an O(n) complexity, there's nothing really you can do here, but this is completely fine as any increases in data size is a linear increase in processing.

\$\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.