4

I am learning jquery and started my first attempt at building a form validation script based on what I know so far (which isnt much).

This is really only the Radio button portion of the validation script, but I thought I get on the right track -coding wise- before I went too far. There are some fundamental issues that I know need addressing.

The Script (jsFiddle): http://jsfiddle.net/pkdsleeper/xNt5n/

The Questions:

a. How best to remove the global variables using 
b. jsLint recommends "use strict", so I added it, but im not sure what it does. 
c. any good refs?
d. Generally, feel free to rip this code apart (cuz I AM trying to learn) but 
   please explain my errors in noob-speak :)

Thanks In advance
sleeper

cssyphus
  • 37,875
  • 18
  • 96
  • 111
sleeper
  • 3,446
  • 7
  • 33
  • 50
  • Updated: Think I got rid of all the Global Vars. Doesnt work in jsFiddle (?), but works locally. http://jsfiddle.net/pkdsleeper/5ta5D/ – sleeper Aug 08 '11 at 00:31
  • 2
    It doesn't work in jsfiddle because you selected the "MooTools" framework. Make sure you select jQuery on the side bar. – Dennis Aug 08 '11 at 01:46
  • It's impossible to choose only ONE right answer as BOTH of these answers are great. We need a way to split'em. – sleeper Aug 08 '11 at 03:46

2 Answers2

3

a. Well, you got rid of the globals pretty well. But as your code looks right now, you can wrap the entire thing in (function(){ ... all your current code in here ... }()) and leave nothing behind in global scope.

b. For "use strict" see this question

c. typeof questions['c'] === "undefined"...

d. Currently, your js is too tied to the markup (html) and vice-versa. If you add or remove something in the form, you'll have to edit your js. If you want to use the validation for another form, you'll have to start the whole thing over again.

Validators are prime candidates for "unobtrusive javascript"/progressive enhancement (here's some material: A List Apart & Wikipedia). Basically, your markup (the html), your styling (the css), and your "behaviors" (the javascript) should all be separate, so they can be changed independently. Here's something you can try:

  1. Add a class name to your form(s) (e.g. "validate")
  2. Set up your js to look for form.validate when the page has loaded
  3. For each form it finds, add a submit event handler
  4. When the handler fires, you search the given form for inputs with various other class names you specify (e.g. "required" or "phone-number" or whatever). The class names tell your code, what kinds of validations should be done. For instance, <input type="text" class="required zip-code"> would mean that a) it's a zip-code, and b) it's a required field.
  5. If there are any validation errors, cancel the submit and alert the user somehow. Otherwise, let the submit proceed.

That's obviously a very rough outline, but I'm not gonna write your code for you :)
The point is, that if you ever need more forms or different forms or something like that you have a generic validation script, and all you need to do is "mark" the inputs with the appropriate class names.

All that being said: There are tons of jQuery plugins out there that do form validation. It's definitely still a good exercise to write one yourself, but it's also a good idea to look at what's already there, and how it works. Don't look at the code right away, but read up on how those validators are used (e.g. do they use class names or something else?) and try to figure out how they work.

Community
  • 1
  • 1
Flambino
  • 18,507
  • 2
  • 39
  • 58
  • Excellent points. Especially on the "theory". I love the classes option. I'm on it! Thanks Again! – sleeper Aug 08 '11 at 03:40
3

a. How best to remove the global variables using

Wrap it in an anonymous function and assign it to the form as a submit listener.

b. jsLint recommends "use strict", so I added it, but im not sure what it does. any good ref's?

Don't bother. It's just a buzz-word for those trying to be hip. You can't use strict mode features on the general web because way too many browsers don't support them. You can use it with otherwise compliant ES 3 code, but it's only useful as a debugging tool for errors that should have been found during testing anyway (e.g. calling constructors without new).

No c?

d. Generally, feel free to rip this code apart (cuz I AM trying to learn) but please explain my errors in noob-speak :)

>   $rdoGroup = [], // an empty array which will be used to hold 

You seem to be using $ to indicate a variable that references a jQuery object, but $rdoGroup is just an array. That may be confusing later.

>     $rdoGroup.push($(this).attr("name"));

The $ function is very expensive, don't use it if you don't need to. Standard HTML attributes are available in all browsers as DOM properties, so:

      $rdoGroup.push(this.name);

Is up to 100 times more efficient, depending on the browser.

>     for (i = 0; i < $rdoGroup.length; i++) {               
>         if ($rdoGroup[c].toString() !== $(this).attr("name").toString()) {

The values assigned to $rdoGroup are strings, calling their toString method is redundant.

As above, use this.name. The name property is a string, so no need for toString.

I think the exercise would be easier without jQuery, which seems to be getting in the way far more than helping. If you are trying to learn javascript, I'd suggest that you learn javascript without using any library.

Once you are reasonably confident with using javascript, then you are far better equipped to use a library for the things the library is good with, and not using it for the things it isn't good at.

RobG
  • 142,382
  • 31
  • 172
  • 209
  • 1
    +1 for suggesting that JS should first be learned without a library involved (I should have said that myself...) – Flambino Aug 08 '11 at 02:36
  • @RobG "The $ function is very expensive, don't use it if you don't need to...." I gotta get my head around this. I understand what your are saying but WHY exactly is it so expensive? Thanks for the code review. Im on it! You guys are the greatest! – sleeper Aug 08 '11 at 03:41
  • 1
    Creating a jQuery object is expensive because it's a function call that returns an object, it does quite a bit of work setting up the object's properties. So only create them when you really need to, and keep references to them if they are to be re-used. Plain DOM methods ar always faster so use them if it's convenient. Here's a quick [dollar-cost](http://jsperf.com/dollar-cost) example on jsperf. – RobG Aug 08 '11 at 04:23
  • Points @RobG for the explanation. and Props on the link to jsperf! I'm gonna ride that puppy like a rented mule :) – sleeper Aug 09 '11 at 08:47