2
\$\begingroup\$

I am writing my own website where users sign up. It then saves that data in an array. I want to know if this is effective or if there are better ways to do it.

var isLoggedOn = false;
var numberOfUsers = 0;
var users = new Array();
var correct = false;
var currentUser = new User();
var date = new Array();
var userNumber = 0;

function getRegistrationFields() {
    var first1 = document.getElementById("firstname");
    first1.onclick = getRegistrationFields;
    var last1 = document.getElementById("lastname");
    last1.onclick = getRegistrationFields;
    var email1 = document.getElementById("email");
    email1.onclick = getRegistrationFields;
    var age1 = document.getElementById("age").value;
    age1.onclick = getRegistrationFields;
    var username1 = document.getElementById("username");
    username1.onclick = getRegistrationFields;
    var password1 = document.getElementById("password");
    password1.onclick = getRegistrationFields;
    var confirm1 = document.getElementById("passwordconfirm");
    confirm1.onclick = getRegistrationFields;
    var first = document.getElementById("firstname").value;
    var last = document.getElementById("lastname").value;
    var email = document.getElementById("email").value;
    var age = document.getElementById("age").value;
    var username = document.getElementById("username").value;
    var password = document.getElementById("password").value;
    var confirmpassword = document.getElementById("passwordconfirm").value;
    var empty = "";

    if ((first === empty) || (last === empty) || (email === empty) || (age === empty)
        || (username === empty) || (password === empty) || (confirm === empty)) {
        var message = "Not all of the fields are filled out";
        alert(message);
        //showModalWindow("fields");
        return false;
    }
    else {
       age = parseInt(age);

       if (password.length < 10) {
            var message2 = "Password must be at least 10 characters long";
            alert(message2);
            //showModalWindow("passwordlength");
            return false;
       } 
       else if (password != confirmpassword) {
            var message3 = "Passwords Do not match";
            alert(message3);
            //showModalWindow("passwordmatch");
            return false;
       }
       else if (age < 18) {
            var message4 = "You must be older to register for this software.";
            alert(message4);
            //showModalWindow("young");
            return false;
       }
       else {
            var message5 = "All of the fields are correct. We will be processing your request";
            alert(message5);
            var d = new Date();
            date[numberOfUsers] = d.toDatestring();
            var user = new User(username, password, email, first, last, age, date[numberOfUsers]);
            users[numberOfUsers]=user;
            numberOfUsers += 1;
            //showModalWindow("success");
            return true;
       }
    }
}


function User(username, password, email, firstname, lastname, age, datecreated) {
   this.username = username;
   this.password = password;
   this.firstname = firstname;
   this.lastname = lastname;
   this.name = firstname + " " + lastname;
   this.age = age;
   this.datecreated = datecreated;
}

Array.prototype.checkFields = function(username, password) {
    for(var i = 0; i < this.length; i++) {
        if(this[i].username === username) {
            if(this[i].password === password) {
                return true; 
                userNumber = i;
            }
         }
         else {
            return false;
        }
    }
}

function CheckSignIn() {
    var username = document.getElementById("username").value;
    var password = document.getElementById("password").value;

    if (username === "" || password === "") { alert("Please enter a username/password"); }
    else {

         if (users.checkFields(username, password)) { 
             window.location = "http://localhost/profile.html";
             currentUser.username = users[userNumber].username;
             currentUser.password = users[userNumber].password;
             currentUser.age = users[userNumber].age;
             currentUser.firstname = users[userNumber].firstname;
             currentUser.lastname = users[userNumber].lastname;
             currentUser.datecreated = users[userNumber].datecreated;
             isLoggedOn = true;
        }
    }
}
\$\endgroup\$
1
\$\begingroup\$

Just a few general notes:

  1. Instead of using arrow if-else { if-else { ... } } structure you could format the code as guard clauses. For example:

    if ((first === empty) || (last === empty) || (email === empty) || (age === empty)
                || (username === empty) || (password === empty) || (confirm === empty)) {
        var message = "Not all of the fields are filled out";
        alert(message);
        return false;
    }
    age = parseInt(age);
    ...
    

    Reference: Flattening Arrow Code

  2. You could create some local variables:

    currentUser.username = users[userNumber].username;
    currentUser.password = users[userNumber].password;
    currentUser.age = users[userNumber].age;
    ...
    

    Here users[userNumber] could be extracted out to a local variable. It would improve readability and remove some duplication.

  3. Using alert does not seem good: "Alert is bad" - really?.

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