refactor form validation js

Greetings,

Check out the following:

<html>
	<head>
		<script type="text/javascript" src="http://ajax.googleapis.com/ajax/libs/jquery/1/jquery.min.js"></script>
	</head>
	<body>
		<form id="theForm" action="process.php">
			<label>Username</label>
			<input type="text" name="username" id="username"/>
			<label>Email</label>
			<input type="text" name="email" id="email"/>
			<label>Password</label>
			<input type="text" name="password" id="password"/>
		</form>
		<input id="submit" type="button" value="submit" />
		<script type="text/javascript">
		$(document).ready(function(){
			Validation ={
				username : $('#username'),
				email : $('#email'),
				password : $('#password'),
				msgChar	:'Each field must have at least 4 characters',
				msgBlank : 'Form fields cannot be left blank',
				msgThanks : 'Thanks for submitting our form!',
				
				validateForm : function(){
					if(Validation.username.val()== ''){
						alert(Validation.msgBlank);
						return false;
					} else if(Validation.username.val()!= '' && Validation.username.val().length <=3){
						alert(Validation.msgChar);
						return false;
					}
					
					if(Validation.email.val()== ''){
						alert(Validation.msgBlank);
						return false;
					}else if (Validation.email.val()!= '' && Validation.email.val().length <=3){
						alert(Validation.msgChar);
						return false;
					}
					
					if(Validation.password.val()== ''){
						alert(Validation.msgBlank);
						return false;
					}else if (Validation.password.val()!= '' && Validation.password.val().length <=3){
						alert(Validation.msgChar);
						return false;
						}
						return true;
					}		 
				};
				
			$('#submit').click(function(){
				if(Validation.validateForm()){
					alert(Validation.msgThanks);
				} else { 
					return false;
				}
			});	
		});
	</script>
	</body>
</html>

Open in new window


Two things: 1) this is just practice I'm doing on my own and 2) the js works as is, but I'm sure it could be re-factored to eliminate all the repeated code areas that are common to each if statement.  I would like see different ways, perhaps better, to coide this.

Thanks.

LVL 1
max7Asked:
Who is Participating?

[Product update] Infrastructure Analysis Tool is now available with Business Accounts.Learn More

x
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

Michel PlungjanIT ExpertCommented:
Several issues, the worst is the submit button is outside the form

I will refactor here:http://jsfiddle.net/mplungjan/AcH6z/

and post the result
0
Michel PlungjanIT ExpertCommented:
Here
<html>
  <head>
    <script type="text/javascript" src="http://ajax.googleapis.com/ajax/libs/jquery/1/jquery.min.js"></script>
    <script type="text/javascript">
    $(document).ready(function(){
      Validation ={
        username : $('#username'),
        email : $('#email'),
        password : $('#password'),
        msgChar  :'Each field must have at least $1 characters',
        msgBlank : 'Form fields cannot be left blank',
        msgThanks : 'Thanks for submitting our form!',
        isBlank:function(fld) {
          if (fld.val() == "") {
            alert(this.msgBlank);
            fld.focus();
            return true;
          }
          return false;  
        },
        isTooShort: function (fld, len) {
          if (fld.val().length < len) {
            alert(this.msgChar.replace(/\$1/,len));
            fld.focus();
            return true;
          }
          return false;
        },   
        validateForm : function(){
          if(this.isBlank(this.username) || this.isTooShort(this.username,4)){
            return false;
          }
          if(this.isBlank(this.email) || this.isTooShort(this.email,4)){
            return false;
          }
          if(this.isBlank(this.password) || this.isTooShort(this.password,4)){
            return false;
          }
          return true;
        }
      }
      
      
      $('#submit').click(function(){
        if(Validation.validateForm()){
          alert(Validation.msgThanks);
          return true;
        }
        return false;
      });  
    });
  </script>
  </head>
  <body>
    <form id="theForm" action="process.php">
      <label>Username</label>
      <input type="text" name="username" id="username"/>
      <label>Email</label>
      <input type="text" name="email" id="email"/>
      <label>Password</label>
      <input type="text" name="password" id="password"/>
      <input id="submit" type="button" value="submit" />
    </form>

  </body>
</html>

Open in new window

0

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
max7Author Commented:
wow.  Night and day, I'd say.  I had thought about using arguments but couldn't think of a way to incorporate it.  And the submit button thing is embarrassing, I just wasn't thinking.

some questions:


1) I notice that with your code, if I put the required 4 characters in a field, I receive the alert msgBlank and then the next field which is empty is highlighted and selected for entered information.  I realize focus() is doing some of that but how it is it that the next empty field is selected?

2) for each function isBlank and isTooShort,  there is a return true and then a return false.  Could you please explain what is going on here i.e. why first return true and then return false?

3) Regarding this line here:

alert(this.msgChar.replace(/\$1/,len));

Open in new window


Why use the replace?  Is that to clean unwanted characters out of whatever is entered in the fields?  If so, is that just standard "good practice" when validating a form?
0
Big Business Goals? Which KPIs Will Help You

The most successful MSPs rely on metrics – known as key performance indicators (KPIs) – for making informed decisions that help their businesses thrive, rather than just survive. This eBook provides an overview of the most important KPIs used by top MSPs.

Michel PlungjanIT ExpertCommented:
1) I focus whatever FIRST field is in error - the return false in each of the validateForm tests makes sure the validation stops at the first error

2) in JavaScript, code after a return is never executed so

isTooShort: function (fld, len) {
  if (fld.val().length < len) {
    alert(this.msgChar.replace(/\$1/,len));
    fld.focus();
    return true; // the field was blank, get out of here and tell the calling function that the field was blank
  }
  return false; // implicit ELSE here
}

3) I decided you might want to ask for 5 characters in another field, so whatever you pass in is used both to test the length AND to construct the message by replacing whereever you put $1 with whatever you fill in.

We could take that one step further and have generic "Please fill in the $1 field" and replace that with fld.name for example
0
max7Author Commented:
>>>We could take that one step further and have generic "Please fill in the $1 field" and replace that with fld.name for example

If it isn't too much to ask, how would that look?
0
Michel PlungjanIT ExpertCommented:
       msgChar  :'The $1 field must have at least $2 characters',
        msgBlank : 'The $1 field cannot be left blank',


       isBlank:function(fld) {
          if (fld.val() == "") {
            alert(this.msgBlank.replace(/\$1/,fld.name));
            fld.focus();
            return true;
          }
          return false;  
        },
        isTooShort: function (fld, len) {
          if (fld.val().length < len) {
            alert(this.msgChar.replace(/\$1/,fld.name).replace(/\$2/,len));
            fld.focus();
            return true;
          }
          return false;
        },  
0
max7Author Commented:
Regarding this code:

$('#submit').click(function(){
        if(Validation.validateForm()){
          alert(Validation.msgThanks);
          return true;
        }
        return false;
      });  

Open in new window


I'm not clear why there is a return true and then, return false.  If the form passes validation, I would assume we would want to stop the script from executing since everything is ok.

Btw, I want you to know that I realize you've answered my initial question and deserve the points to be awarded.  I'm hoping you don't mind that I dig deeper on these since I really want to understand it better.
0
Michel PlungjanIT ExpertCommented:
No problem

return true will submit the form, return false will stop the submission
you need BOTH statements

$('#submit').click(function(){
        if(Validation.validateForm()){
          alert(Validation.msgThanks); // the validation passed
          return true; // allow the submission
        } // implicit else
        return false; // disallow submission
      });  
0
Michel PlungjanIT ExpertCommented:
ACTUALLY I would prefer


$('#theForm').submit(function(e){ // pass the event
  if(Validation.validateForm()){
    alert(Validation.msgThanks); // the validation passed
    return true; // allow the submission
  } // implicit else
  e.preventDefault(); // stop the default event
  return false; // disallow submission (for old time's sake)
});  
0
max7Author Commented:
Sorry for letting this question languish; I've gotten sucked into a project but I still have perhaps a couple of more questions forthcoming before i close it out?
0
Michel PlungjanIT ExpertCommented:
No problems
0
max7Author Commented:
regarding this code:

 msgChar  :'The $1 field must have at least $2 characters',
        msgBlank : 'The $1 field cannot be left blank',


       isBlank:function(fld) {
          if (fld.val() == "") {
            alert(this.msgBlank.replace(/\$1/,fld.name));
            fld.focus();
            return true;
          }
          return false;  
        },
        isTooShort: function (fld, len) {
          if (fld.val().length < len) {
            alert(this.msgChar.replace(/\$1/,fld.name).replace(/\$2/,len));
            fld.focus();
            return true;
          }
          return false;
        },  

Open in new window


fld.name is coming back undefined.  I augmented the code in the following manner to troubleshoot:

validateForm : function(){
          if(this.isBlank(this.username) || this.isTooShort(this.username,4)){
          	alert(this.username.toSource());
            return false;
          }
          if(this.isBlank(this.email) || this.isTooShort(this.email,4)){
          	alert(this.email.toSource());
            return false;
          }
          if(this.isBlank(this.password) || this.isTooShort(this.password,4)){
          	alert(this.password.toSource());
            return false;
          }
          return true;

Open in new window


and along with the message "The undefined field must have at least 4 characters", I then get "({length:1, 0:({}), context:({}), selector:"#username"})"

So that's two alerts in total but I'm not clear on why fld.name would be undefined.
0
Michel PlungjanIT ExpertCommented:
fld.attr("name") would likely work better than fld.name. Apologies for regressing to normal JS syntax

Can you revert to my code and try that instead?
0
max7Author Commented:
Yes that worked.  What threw me off is that I believe in certain instances, I've been able to use normal js syntax with jQuery and it has worked ... or at least, I think that is the case.
0
max7Author Commented:
regarding this code:

$('#theForm').submit(function(e){
        if(Validation.validateForm()){
          alert(Validation.msgThanks);
          return true;
        }
         e.preventDefault();
        return false;
      });  

Open in new window


unless I've made an error on my end, it's not working.  I click the submit button and none of the js runs at all.
0
Michel PlungjanIT ExpertCommented:
Jquery allows normal access via "this" instead of $(this) I'll look when I get to a computer
0
max7Author Commented:
Ok, the type was wrong on the submit input.  Once corrected, it submitted the form just fine.

Testing a couple of other things now ...
0
max7Author Commented:
back to this code you rewrote:

$('#theForm').submit(function(e){
        if(Validation.validateForm()){
          alert(Validation.msgThanks);
          return true;
        }
         e.preventDefault();
        return false;
      });

Open in new window


As I said, it works but then I reverted back to my original code:

$('#theForm').submit(function(){
        if(Validation.validateForm()){
          alert(Validation.msgThanks);
       }
        return false;
      });  

Open in new window


and this appears to produce the same end result as your code.  

My question is: what is the advantage of using your code over mine?  Why pass the event and then prevent the default event (rather than using return false)?  My experience with this coding (passing the event, preventing default event) is limited.
0
Michel PlungjanIT ExpertCommented:
Just me wearing suspenders and a belt. Returning false should be enough
0
max7Author Commented:
HA! alrighty then :)

but my question is: why would someone want to use function(e) and then e.preventDefault(); instead of simply return false; ?
0
Michel PlungjanIT ExpertCommented:
Because that seems to be the jQuery cross browser way to do it. I'll investigate it for you tomorrow
Bed time here
0
Michel PlungjanIT ExpertCommented:
0
max7Author Commented:
wow, that looks awesome.  Thanks so much.  I guess I shall cease my interrogation of you on this question :)
0
max7Author Commented:
Thank you so much for your generous help, I learn so much from you.
0
Michel PlungjanIT ExpertCommented:
You are welcome..

I had to read that article too since I was not 100% sure on the issue so you helped me too :)
0
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
JavaScript

From novice to tech pro — start learning today.