HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptModerate

Is this form submission validator professional?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thisprofessionalvalidatorformsubmission

Problem

I have written some jQuery code for AJAX form submission:

jQuery(function() {
    jQuery('#message').click(function() {
        var btn = jQuery(this)
        var Name = jQuery("#Name").val();
        var Email = jQuery("#Email").val();
        var Sub = jQuery("#Sub").val();
        var Message = jQuery("#Message").val();
        if (Name == "") {
            jQuery('#Name').focus();
            jQuery("#Name").after('');
        }
        else if (Email == "") {
            jQuery("#Email").focus();
            jQuery("#Email").after('');
        }
        else if (Sub == "") {
            jQuery("#Sub").focus();
            jQuery("#Sub").after('');
        }
        else if (Message == "") {
            jQuery("#Message").focus();
            jQuery("#Message").after('');
        }
        else if (!validateEmail(Email)) {
            jQuery("#Email").focus();
        }
        else {
            btn.button('loading')
            $.ajax({
                url: '/application',
                dataType: 'text',
                type: 'post',
                data: {querytype: 'form', name: Name, email: Email, sub: Sub, message: Message},
                success: function(data)
                {
                    jQuery('#success').fadeIn();
                }
            });
        }
    });
});


Is this code good to go? Or it can be improved? I have seen this where they check the validation after form submission, or this.

Should I check validation before or after submission?

Will my approach be good if I need to handle 10+ input elements?

Solution

A few things:

  • Variables in JavaScript should be camelCased. PascalCase is traditionally left for types.



  • CSS classes are generally lower-case and hyphenated. Instead of #Name, consider #name (and the corresponding HTML attribute, id='name')



  • Use $ instead of jQuery. $ is syntactic sugar that essentially resolves the same thing, but it's one character instead of 6, and it's more conventionally used (see below)



  • Consider using the jquery.validate library. This will make your validation significantly less painful. If you're using ASP MVC you can use jquery.validate.unobtrusive to automatically map your validation on your view model from C# to JavaScript.



  • Keep your semi-colon conventions consistent. Use them at the end of every statement, or don't use them at all.



  • If you're going to use $('#element') calls a lot, consider calling that $('#element') once and storing it inside of a variable inside the scope you are using that element.



With regards to validation, you should always perform validation on the server side. I cannot overstate that point. You must do it. Client-side validation is a bonus that should be performed in addition to server-side validation, but should not be relied upon and you should not make it core functionality to your site (users can easily disable JavaScript, and some browsers like Tor have JavaScript disabled by default).

Remember that the golden rule in programming is that end-users are stupid. The weakest point to any application is the point that the end-user is allowed to provide input. You should treat the data received from the form as such, even if you already have client-side validation.

Your approach will work with 10+ input elements, but you will be repeating a lot of code. As suggested above it's advisable for you to check out jquery.validate which will allow you to maintain validation rules a lot easier (and DRY up your validation)

I don't think you should be concerned whether or not your code is professional but rather that it signals it's intent to other developers clearly and it is maintainable. Your code is certainly clear in what it tries to do it but strikes me as scaling very badly in terms of maintainability, and I think you should address that.

Example of jQuery Validate

Well, I've only really used jquery.validate.unobtrusive, but here's a link to the library. I'm sorry I can't provide much more info on that, as most of it was done automatically for me behind the scenes by ASP MVC 4.

jQuery in WordPress


Wordpress automatically no-conflicts jQuery as a matter of courtesy. If you're only using jQuery, you can override this and just put in a $ = jQuery; somewhere. Better yet, you can use jQuery(function($) { ... to alias $ to jQuery inside your ready handler. – Schism

Or, as pointed out by @Pinoniq in his answer, you can wrap it in an IIFE (aka self-executing functions).

Context

StackExchange Code Review Q#61830, answer score: 17

Revisions (0)

No revisions yet.