Reputation: 4390
I have a form that has five fields; first name
, last name
, username
, password
and passwordConfirmation
.
I'd firstly like to say that I don't want the most complicated method, this is for a very simple website that will never go live, it is purely for demonstration purposes and I am not to worry about error catching in the sense of harmful user input.
So far I have this:
function validateRegistrationForm(firstName, lastName, username, password, passwordConfirmation) {
alert("validate form function has been called");
//alert("function has been called");
//---
var firstName = firstName;
//alert(firstName);
var lastName = lastName;
//alert(lastName);
var username = username;
//alert(username);
var password = password;
//alert(password);
var passwordConfirmation = passwordConfirmation;
//alert(passwordConfirmation);
//---
//**The code that I have sectioned off here, I am unsure whether I need it or not, could someone please explain to me why I would do this, and not just reference the parameters in the function directly?**
// Check here if all the fields in the registeration form have been filled.
if ( firstname == null || firstname == "" ) {
//firstname hasn't been filled out
}
if ( lastName == null || lastName == "" ) {
//lastname hasn't been filled out
}
if ( username == null || username == "" ) {
//username hasn't been filled out
}
if ( password == null || password == "" ) {
//password hasn't been filled out
}
if ( passwordConfirmation == null || passwordConfirmation == "" ) {
//passwordconfirmation hasn't been filled out
}
}
I would like to have a way to check if each of the fields have been filled out (which I have here I think) and if not, to produce a notification such as a red border around the input element in question and/or a message with instructions on how to resolve the issue. I know this can be done with css but I am not sure of the most efficient way to do it.
Currently I have created these two functions:
function generateError(messageText, elementIdentifier) {
alert("error generator called");
//var element = document.getElementById(elementID);
//var error = document.createTextNode(message);
//element.innerHTML = message;
alert("Element: " + elementIdentifier + ", Reason: " + messageText);
}
function validatePassword(password, passwordConfirmation) {
alert("password check called");
if (password == passwordConfirmation) {
if (password.length < 8) {
//password too short
//alert("password\'s match but they are too short");
return false;
} else {
//passwords match and the right length
//alert("password\'s match and they are 8 characters or longer");
return true;
}
} else {
//the two do not match
//alert("they don\'t match")
generateError("Passwords don\'t match", "password");
return false;
}
}
The validatePassword()
function takes the password and the password confirmation as two parameters, I think this works already, what I want is for the errors generated to be passed to the generateError()
function which stores them all in an array and then loops through them one by one displaying to the user what is wrong.
Upvotes: 0
Views: 405
Reputation: 11765
Define complicated. I think that your code is already too complicated. The giveaway is that there is repetition. Whenever you see repetition, ask yourself if there is a way to combine all that repetition into a loop. The result is less code but usually also code that is more friendly to change. If you want to add a phone number, for example, you'd have to add four more lines of code as well as another parameter, which will break your function's backward compatibility unless you add even more code allowing for an undefined phone number. You might say "I'm definitely not adding any more fields, so that isn't an issue." I don't know how many times I have said that myself and I ended up having to eat my words and do surgery on my functions. Besides, its just better engineering.
Also, let's revisit your validateRegistrationForm function. The way you have it now, my guess is that it is triggered by onsubmit, but why not take advantage of JavaScript's snappiness by giving the user instant feedback on each field? I'm going to change it to validateRegistrationElement and it will be called onchange. We are only going to pass in one parameter, the textbox being evaluated. That way we can use "this". So each textbox will look something like:
<input type="text" id="firstname" onchange="validateRegistrationElement(this)" />
Also, next to each, let's put a feedback box next to each textbox and get rid of those annoying alert popups. We can even style our feedback box with CSS classnames "error" and "success" and make them red or green, respectively.
<div id="firstnameFeedback"></div>
...and so forth.
The CSS:
.error {background-color: pink; border: 1px dashed red; color: white}
.success {border: 1px dashed green;}
Instead of loading up our function with the details of each form element, let's separate the instructions into a sort of config variable, an array of objects. Each member of the array will represent one form element, so in your case there would be four members of the array. These would be objects which contain everything the function needs to know to report back an error. My suggestion is to let each object contain the id of the form element, a label, a requirement (an array of regular expressions paired with their own feedback messages would be ideal, but let's keep it simple with string length), and a special condition for the passwords matching.
var myFields =
[
{
"id": "firstname",
"label": "first name",
"minLength": 1 //in other words, the field is required
},
{
"id": "lastname",
"label": "last name",
"minLength": 1
},
{
"id": "password1",
"label": "first password",
"minLength": 6,
"maxLength": 8,
"mustMatch": "password2"
},
{
"id": "password2",
"label": "second password",
"minLength": 6,
"maxLength": 8,
"mustMatch": "password1"
},
]
Is this too complicated? I don't think so. Now we have a bucket of legos that our validator function can reach into. We can add or subtract elements easily. We can even add features for later if we want and it won't break the function (like a password maxlength, which I have left out of the function). I'll show you. Here is the new validator function. Remember that this is called whenever the user changes a value in a field. The function will know which one because we used "this" in our html.
function validateRegistrationElement(field) {
var message = ""; //declare an empty string literal for the message
for (i=0;i<myFields.length; i++){ //loop through each array element until we find the object
if(myFields[i].id == field.id){ //once we've found the config object
if(myFields[i].minLength > field.value.length){ //if the field is shorter than what's allowed
message += myFields[i].label + " must be longer than " + myFields[i].minLength;
}
if (typeof(myFields[i].mustMatch) != 'undefined'){ //if the field must match another...
if(field.value != document.getElementById(myFields[i].mustMatch)){ //...does it match
message += myFields[id].label +" must match "+myFields[id].mustMatch;
}
}
document.getElementById(field.id + "Feedback").innerText = message;
if (message.length > 0) {setClass(field.id, "error")} //if there an error message, highlight red
else{setClass(field.id, "success")}//otherwise, highlight green
}
}
}
Notice that this also takes care of the passwords so you don't need an extra function for them.
This function manages the CSS and is called by the above function.
function setClass(id, className) {
document.getElementById(id).className = "";
document.getElementById(id).className = className;
document.getElementById(id+"Feedback").className = "";
document.getElementById(id+"Feedback").className = className;
}
Upvotes: 1
Reputation: 2624
Try great jQuery Validation plugin.
You will need little more markup (see demos and docs) and just $("#yourForm").validate();
.
Upvotes: 0