Can anyone please tell me how I can improve this code, and most importantly sort my Email validation to work in all cases where Z does not either equal nothing or string "Email"
All fields start with appropriate wording already entered as an example to customers.
function validateForm()
{
//Uses HTML field IDs
var x=document.forms["myForm"]["name"].value;
var y=document.forms["myForm"]["phone"].value;
var z=document.forms["myForm"]["email"].value;
//Name locator
if (x==null || x=="" || x=="Name")
{
alert("Please enter the your name.");
return false;
}
//Contact method locator
if ((y==null || y=="" || y=="Phone Number")&&(z==null || z=="" || z=="Email"))
{
alert("Please enter a contact method.");
return false;
}
//Phone numeric validation, this runs if Email field is not edited
if (z==null || z=="" || z=="Email")
{
if (isNaN(y)||x.indexOf(" ")!=-1)
{
alert("Telephone must be a numeric value.");
return false;
}
}
//Phone length validation, this runs if Email field is not edited
if (z==null || z=="" || z=="Email")
{
if (y.length > 14)
{
alert("Telephone must be valid.");
return false;
}
}
//Email validation, does not work, this should run only when something is entered into the field
if (z!=null || z!="" || z!="Email")
{
var atpos=z.indexOf("@");
var dotpos=z.lastIndexOf(".");
if (atpos<1 || dotpos<atpos+2 || dotpos+2>=z.length)
{
alert("This is not a valid e-mail address");
return false;
}
}
}
Can anyone please tell me how I can improve this code, and most importantly sort my Email validation to work in all cases where Z does not either equal nothing or string "Email"
All fields start with appropriate wording already entered as an example to customers.
function validateForm()
{
//Uses HTML field IDs
var x=document.forms["myForm"]["name"].value;
var y=document.forms["myForm"]["phone"].value;
var z=document.forms["myForm"]["email"].value;
//Name locator
if (x==null || x=="" || x=="Name")
{
alert("Please enter the your name.");
return false;
}
//Contact method locator
if ((y==null || y=="" || y=="Phone Number")&&(z==null || z=="" || z=="Email"))
{
alert("Please enter a contact method.");
return false;
}
//Phone numeric validation, this runs if Email field is not edited
if (z==null || z=="" || z=="Email")
{
if (isNaN(y)||x.indexOf(" ")!=-1)
{
alert("Telephone must be a numeric value.");
return false;
}
}
//Phone length validation, this runs if Email field is not edited
if (z==null || z=="" || z=="Email")
{
if (y.length > 14)
{
alert("Telephone must be valid.");
return false;
}
}
//Email validation, does not work, this should run only when something is entered into the field
if (z!=null || z!="" || z!="Email")
{
var atpos=z.indexOf("@");
var dotpos=z.lastIndexOf(".");
if (atpos<1 || dotpos<atpos+2 || dotpos+2>=z.length)
{
alert("This is not a valid e-mail address");
return false;
}
}
}
-
1
Have you tried to
alert(z)to see what it's value is before the validation? – DevlshOne Commented Aug 22, 2013 at 14:31 -
2
Some tips: a) Do not ever name variables
x, y, zif they don't represent vector data or the like. b) Correct your indentions. c) Keep your code DRY: create a function whichs checks fornull,""or a given value (e.g. "Name", "Email", "Phone Number"). – ComFreek Commented Aug 22, 2013 at 14:32 - 1 Your condition will be true when z is null or z is empty string or z equals "Email". Doesn't it work this way for you? What exactly is the problem? Now if you want an advice on improvement, use Placeholder attribute, then you won't have to check for "Email": w3schools./tags/att_input_placeholder.asp – Sergey Snegirev Commented Aug 22, 2013 at 14:33
- Cheers for the reply, alert(z) returns, "Email", the default value. It runs the method regardless. I need the condition to be true only when z does not equal null, empty string or "Email". – user2707736 Commented Aug 22, 2013 at 14:37
- 1 Shouldn't it be && instead of || – Nigel B Commented Aug 22, 2013 at 14:45
4 Answers
Reset to default 5You could do a few things.
z==null || z=="" could be replaced with !Boolean(z) or !z
z!=null || z!="" could be replaced with Boolean(z) or !!z
You should also try to always use === instead of == unless your expecting type coercion.
So your check for z == "Email" could change to something like this z.toLowerCase() === "email"
It also seems like you repeat code --> z==null || z=="" || z=="Email" (x2). You could bine Phone numeric validation and Phone length validation.
Take a look at this validation code which is a bination of jQuery and Javascript. It will shed some excellent insight on what you should be looking for and how to process specific elements of a typical form (including a GREAT RegEx example for email address:
/^((([a-z]|\d|[!#\$%&'\*\+\-\/=\?\^_`{\|}~]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])+(\.([a-z]|\d|[!#\$%&'\*\+\-\/=\?\^_`{\|}~]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])+)*)|((\x22)((((\x20|\x09)*(\x0d\x0a))?(\x20|\x09)+)?(([\x01-\x08\x0b\x0c\x0e-\x1f\x7f]|\x21|[\x23-\x5b]|[\x5d-\x7e]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(\\([\x01-\x09\x0b\x0c\x0d-\x7f]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF]))))*(((\x20|\x09)*(\x0d\x0a))?(\x20|\x09)+)?(\x22)))@((([a-z]|\d|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(([a-z]|\d|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])*([a-z]|\d|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])))\.)+(([a-z]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(([a-z]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])*([a-z]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])))$/i.test(value)
Change:
if (z!=null || z!="" || z!="Email")
To this:
if (z!=null && z!="" && z!="Email")
Your validation should all be ANDs vs ORs since you want all of that to be true before submitting.
Rather than manually writing form validation, have you considered using something like validate.js?
Example code:
var validator = new FormValidator('example_form', [{
name: 'req',
display: 'required',
rules: 'required'
}, {
name: 'alphanumeric',
rules: 'alpha_numeric'
}, {
name: 'password',
rules: 'required'
}, {
name: 'password_confirm',
display: 'password confirmation',
rules: 'required|matches[password]'
}, {
name: 'email',
rules: 'valid_email'
}, {
name: 'minlength',
display: 'min length',
rules: 'min_length[8]'
}], function(errors, event) {
if (errors.length > 0) {
// Show the errors
}
});