Sanitizing and validating a user signup form before submission The first part of the code (above the in-code comment in the middle) looks messy.

I know I could use variables to make it cleaner, I just don't exactly know yet how can I effectively apply those when dealing with jQuery objects. So I'd appreciate a review on that.

As for the if-statements, I think they're okay but if they could be improved I'm all open for it.

Any other suggestions are always welcome.

$(function() {
    // magic number
    var MAX_FIRST_NAME_LENGTH = 35;

    // Hide additional input messages by default except errors.
    $("#signup .addition:not(.error)").css("display", "inline-block").parent().hide();

    // Select (focus) first input and show it's additional (erroneous) message.
    if ($("#signup .error").length === 0) {
        $("#signup input[name=firstName]").select();
        $("#signup input[name=firstName]").siblings("#signup div").show();
    } else if ($("#signup .error").first().parent("#signup div").siblings($("#signup input[name=firstName]")).val().length &gt;= 1) {
        if ($("#signup input[name=firstName]").val().length &gt; MAX_FIRST_NAME_LENGTH) {
            $("#signup input[name=firstName]").select();
        } else {
            $("#signup input[name=lastName]").select();
        }
    } else {
        $("#signup .error").first().parent("#signup div").siblings($("#signup input")).select();
    }

    // Toggle slide on the additional message focus / blur of corresponding input.
    $("#signup input").on("focus blur", function(e) {
        if (!$(this).siblings("#signup div").children("#signup .addition").hasClass("error")) {
            $(this).siblings("#signup div").stop()[e.type === "focus" ? "slideDown" : "slideUp"]("fast");
        }
    });

/// I think the code below is good as is.

    var $form = $("#signup");

    $form.on("submit", function() {
        var emptyInputs = 0;

        $form.find("input").each(function() {
            var $this = $(this),
                val   = $this.val();

            if ($this.prop("type") !== "password") {
                val = $.trim(val);
            }

            // Focus (select) first empty field if any.
            if (val === "" &amp;&amp; !emptyInputs++) {
                $this.select();
            }
        });

        if (emptyInputs === 0) {
            var $button = $("#signup button");

            $button.prop("disabled", true);

            return true;
        }

       return false;
    });
});
Kid Diamond! It's always great to see code improving over time. ☺

On loading, you treat input[name=firstName] and input[name=lastName] specially, due to the need for some specialised logic. Instead of selecting each of them every time, you can assign them to variables. The variables are prefixed with $ to remind you that these are jQuery objects.

var $firstName = $("#signup input[name=firstName]"),
    $lastName  = $("#signup input[name=lastName]");


In the first if block, you could optionally take advantage of method chaining. I get rid of the #signup in the selector: since $firstName is already a descendant of #signup, then its siblings will be too, and so it's unnecessary.

$firstName.select().siblings("div").show();


The else if condition is not immediately obvious. After looking at it more carefully, the logic flow is this:


Consider whether the error is associated with $firstName.


If so, consider whether $firstName is not empty.


If $firstName is not empty, then consider whether it is greater than the max length.


If so, then $firstName is the errant field and should be selected.
Otherwise, $firstName is fine, so $lastName must be errant and selected.

If $firstName is empty, then fall-back to the else (below).

Else, select the input associated with the first error.



This is actually rather complicated. In fact, for this snippet, I prefer your original code. Here is how I would rewrite it:

var $errors = $("#signup .error");
if ($errors.length === 0) {
    $firstName.select().siblings("div").show();
} else {
    var $firstError = $errors.first();
    if ($firstError.closest(".input").has($firstName).length &gt; 0) {
        if (!$firstName.val() || $firstName.val().length &gt; MAX_FIRST_NAME_LENGTH) {
            $firstName.select();
        } else {
            $lastName.select();
        }
    } else {
        $firstError.closest(".input").children("input").select();
    }
}


In the event handler, you can at least do var $div = $("#signup div"). Personally, I'd rather rewrite like the following, just because the logic reads slightly more clearly to me:

$("#signup input").on("focus blur", function(e) {
    var $div = $(this).siblings("#signup div");
    if ($div.has(".error").length &gt; 0) {
        $div.stop();
        if (e.type === "focus") {
            $div.slideDown("fast");
        } else {
            $div.slideUp("fast");
        }
    }
});


Below the comment, the code is pretty good. The one thing I'd change is I wouldn't make a $button variable at all, since it's only used once. I might also reverse the order of the logic to avoid the unnecessary return true;:

if (emptyInputs &gt; 0) {
    return false;
}
// if the form submits successfully, disable the button to prevent accidental double-submits.
$("#signup button").prop("disabled", true);