3
\$\begingroup\$

Simple really my javascript function for checking my form.

var errors = {};
errors.email = true;
errors.cemail = true;
errors.password = true;
errors.cpassword = true;
errors.username = true;
function joinAjax (id) {
var val = $('#' + id).val();
if (id == 'email') {
 $('#emailMsg').hide();
 var reg = /[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?/;
 if (val == '') {
 $('#' + id).after('<div id="emailMsg" class="error">Enter your email</div>');
 }
 else if (reg.test(val) == false) {
 $('#' + id).after('<div id="emailMsg" class="error">Email invalid</div>');
 }
 else {
 errors.email = false;
 $('#' + id).after('<div id="emailMsg" class="success">Email OK</div>');
 }
 joinAjax('cemail');
}
if (id == 'cemail') {
 $('#cemailMsg').hide();
var email = $('#email').val();
 if (errors.email == false) {
 if (val != email) {
 $('#' + id).after('<div id="cemailMsg" class="error">Emails do not match</div>');
 }
 else {
 errors.cemail = false;
 $('#' + id).after('<div id="cemailMsg" class="success">Success</div>');
 }
 }
 else {
 $('#cemail').val('');
 }
}
if (id == 'password') {
 $('#passwordMsg').hide();
 if (val == '') {
 $('#' + id).after('<div id="passwordMsg" class="error">Enter a password</div>');
 }
 else if (val.length < 6) {
 $('#' + id).after('<div id="passwordMsg" class="error">Minimum length of 6</div>');
 }
 else {
 errors.password = false;
 $('#' + id).after('<div id="passwordMsg" class="success">Password OK</div>');
 }
 joinAjax('cpassword');
}
if (id == 'cpassword') {
 $('#cpasswordMsg').hide();
 var password = $('#password').val();
 if (errors.password == false) {
 if (val != password) {
 $('#' + id).after('<div id="cpasswordMsg" class="error">Passwords do not match</div>');
 }
 else {
 errors.cpassword = false;
 $('#' + id).after('<div id="cpasswordMsg" class="success">Success</div>');
 }
 }
 else {
 $('#cpassword').val('');
 } 
}
if (id == 'username') {
 $('#usernameMsg').hide();
 if (val == '') {
 $('#' + id).after('<div id="usernameMsg" class="error">Enter a username</div>');
 }
 else if (val.length < 3) {
 $('#' + id).after('<div id="usernameMsg" class="error">Minimum length is 3</div>');
 }
 else {
 errors.username = false;
 $('#' + id).after('<div id="usernameMsg" class="success">Available</div>');
 }
}
$('#joinForm').submit(function(event){
 if ((errors.email == true) || (errors.cemail == true) || (errors.password == true) || (errors.cpassword == true) || (errors.username == true)) {
 event.preventDefault();
 }
 return true;
});
}

Can this be improved? I was looking for a way to create a function for the submit event, but I can't figure it how as I don't know how many parameters I will need each time.

asked Sep 23, 2011 at 16:03
\$\endgroup\$

3 Answers 3

3
\$\begingroup\$

Some tips:

answered Sep 26, 2011 at 6:38
\$\endgroup\$
3
\$\begingroup\$

I see a few references to $('#' + id), it may be easier to maintain and also save some memory usage if you declare a variable and set it to that value.

var form_id = $('#' + id);

Then anywhere you reference $('#' + id), just put in form_id.

answered Sep 26, 2011 at 13:25
\$\endgroup\$
2
\$\begingroup\$

A few things I'll recommend:

Turn lines 1 - 7 into an object literal, it makes the intent a bit clearer.

var errors = {
 email : true,
 cemail : true,
 password : true,
 cpassword : true,
 username : true
};

Use the non type-coersion comparison operators === and != instead of == when evaluating false and numeric values.

So change line 33 to:

if (errors.email === false) {

Encapsulate your error checking into separate functions, one example would be something like:

if (id == 'cemail') {
 checkEmail();
}
function checkEmail(){
 $('#cemailMsg').hide();
 var email = $('#email').val();
 if (errors.email === false) {
 if (val != email) {
 $('#' + id).after('<div id="cemailMsg" class="error">Emails do not match</div>');
 }
 else {
 errors.cemail = false;
 $('#' + id).after('<div id="cemailMsg" class="success">Success</div>');
 }
 }
 else {
 $('#cemail').val('');
 }
}

Then once you get to that level of encapsulation and granularity you can start to add out functions for adding your error messages so you aren't duplicating that code. Off the top of my head something like:

if (val != email) {
 addError(id,'<div id="cemailMsg" class="error">Emails do not match</div>');
 }
function addError(divId,error){
 $('#' + divId).after(error);
}

This way if you change how you handle errors you can do it in one place.

answered Sep 30, 2011 at 11:50
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.