Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

In a project, I have a lot of HTML forms without validation and I want to add a front end validation, in a quick way. I start looking for a library and I met Parsley.js. It's a good library, but if I decided to use it I would have to modify all the forms in my application.

So there is where this code is born. It's for automatically binding a set of rules you define to a form.

//Only basic types and validators.
$.fn.parsley.mapping = {
//Types
'alphanum': {'parsley-type':    'alphanum'},
'email':    {'parsley-type':    'email'},
'url':  {'parsley-type':    'url'},
'number':   {'parsley-type':    'number'},
'digits':   {'parsley-type':    'digits'},
'dateIso':  {'parsley-type':    'dateIso'},
'phone':    {'parsley-type':    'phone'},

//Validators
'required': {'parsley-required':    'true'},    
'notblank': {'parsley-notblank':    'true'},
'minlength':    {'parsley-minlength': '?'},
'maxlength':    {'parsley-maxlength': '?'},
'min':  {'parsley-min': '?'},   
'max':  {'parsley-max': '?'}
}

//Only inputs and textareas
$.fn.parsley.fields = 'input:visible, textarea:visible';

!function ($) {
$.fn.generalValidation = function( validationRules ) {

var name, validations, validationName, validationValue, att, key, value
, form = $( this )
, addValidations = function( validationRules ){
if(validationRules === undefined){
console.log('Error!!! You need to add the validation rules parameter');
validationRules = {};
}
$( form ).find( $.fn.parsley.fields ).each(function(){
name = $( this ).attr('name');
validations = validationRules[name];

if(validations == undefined) return true;

for(validation in validations){
validationName = validations[validation];
validationValue = '';
if(validationName.indexOf(':') !== -1){
validationValue = validationName.substr(validationName.indexOf(':')+1, validationName.length);
validationName = validationName.substr(0, validationName.indexOf(':'));
}

att = $.fn.parsley.mapping[validationName];
if(att !== undefined){
$.map(att, function (v, k) { key = k; value = v; });
value = (value === '?') ? validationValue : value;
$(this).attr(key, value);
}
}
});
return true;
}
, callParsley = function( form ) {
form.parsley();
};

if(addValidations( validationRules )){
callParsley( form );
}
}
} ( window.jQuery );

Here is a JSFiddle.

This is my first plugin in jQuery. Would you improve something in the code? Do you find this useful?

share|improve this question
    
I'd recommend keeping the old code embedded here. That's more important than keeping the new code as the answers are based on the old code. –  Jamal Feb 19 at 2:40
    
Please avoid invalidating existing answers by modifying your question with updated code. Let the answers sink in, refactor as needed, and then post the updated code in a new question. More votes for everyone! :) –  Mat's Mug Feb 19 at 2:40
    
@lol.upvote Thanks for pointing this. I'm going to take it into consideration next time. (I know I shouldn't say thanks :P) –  gonzalon Feb 19 at 2:46
add comment

2 Answers

up vote 3 down vote accepted

From a once over:

  • The following is not DRY at all, you have to find a way to not do this:

    //Types
    'alphanum': {        'parsley-type': 'alphanum'    },
    'email': {        'parsley-type': 'email'    },
    'url': {        'parsley-type': 'url'    },
    'number': {        'parsley-type': 'number'    },
    'digits': {        'parsley-type': 'digits'    },
    'dateIso': {        'parsley-type': 'dateIso'    },
    'phone': {        'parsley-type': 'phone'    },
    

    I am pretty sure you can throw all that way by changing to the below statement:

    att = $.fn.parsley.mapping[validationName] || {  'parsley-type': validationName  };
    
  • Also, $.fn.parsley.mapping could use some aligning to look better
  • You are calling $.map without capturing the array it returns, perhaps you meant to use forEach ? Also, it seems that your code will not work correctly if there is more than 1 property in parsley.mapping.You keep assign to the same key and value.
  • Your var statement is too messy:

    var name, validations, validationName, validationValue, att, key, value, form = $(this),
        addValidations = function (validationRules) {
    
  • Comparisons with undefined should be either with === or by doing a falsey check so use either if(validations === undefined) or if(!validations)
  • There is no var validation for for(validation in validations){
  • There are plenty of comma and semicolon problems, you should use jshint to find & solve them

All in all, I like the value of your wrapper, but I think it needs some more polishing.

share|improve this answer
    
@konjin I just update my question with the new code following some of your suggestions and JSHint. I can't figure it out how to get rid of the map function in the for, both (you and JSHint) don't like it haha. –  gonzalon Feb 19 at 2:39
add comment

What about jquery validate? You shouldn't have to change any forms... If you're changing the form to accommodate the JavaScript you're not doing it right.

share|improve this answer
    
Hey, all the plugins or whatever I looked you have to modify your form and add the rules for the validation. Can you point me out one that does not require that? (Googled jquery validate but I don't know what are you referring) –  gonzalon Feb 19 at 1:48
    
Read the first two paragraphs: jqueryvalidation.org/documentation. You can set the rules in an object that is passed to validate () rather than in the form. –  Darius Feb 19 at 5:02
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.