Function to show or hide validation error messages

Question

I'm a bit clumsy at writing JavaScript. I have some nice code like this:

(function($){
    $.fn.forceNumeric = function(){
        return this.each(function(){
            $(this).keyup(function(){
                if (!/^[0-9]+$/.test($(this).val())){
                    $(this).val($(this).val().replace(/[^0-9]/g, ''));
                }
            });
        });
    };
})(jQuery);

I know what it does and how it works. I can call the function and I see why it works.

For the life of me, I cannot figure out how to use that same elegant style/approach for my Neanderthal function:

function showHideErrorBox(theFriendlyName, check){
     var topLevelDiv,
        numDivsAfter;

        if(check === 0){
        // REMOVE THE OLD ERROR MESSAGE IN THE ERROR BOX
            $('.form-validation-errors').find('div').each(function(){
                if( $(this).html().indexOf(theFriendlyName) != -1 ){
                    $(this).remove();
                }
            });
        }

        //  CHECK WHETHER THE ENTIRE MESSAGE BOX CAN BE REMOVED NOW
        topLevelDiv = document.getElementById('alert-form-errors');
        numDivsAfter = topLevelDiv.getElementsByTagName('div').length;

        if(check === 0){
            if(numDivsAfter <= 1){
                $('#alert-form-errors').hide();
            }

        }else if(check === 1){

            if(numDivsAfter >= 1){
                $('#alert-form-errors').show();
            }    
        }
}

Can anyone help me out? Where do I start? I have Google'd for tutorials, but I have not been able to find something suitable for my needs.

Answer 1

You could start by minimizing your function calls. like so

ORIGINAL

$.fn.forceNumeric = function(){
    return this.each(function(){
        $(this).keyup(function(){
            if (!/^[0-9]+$/.test($(this).val())){
                $(this).val($(this).val().replace(/[^0-9]/g, ''));
            }
        });
    });
};

OPTIMIZED

$.fn.forceNumeric = function(){
    return this.each(function(){
        $(this).keyup(function(){
            if (!/^[0-9]+$/.test(this.value)){
                this.value = this.value.replace(/[^0-9]/g, '');
            }
        });
    });
};

Just in this little bit you have saved on 3 Function calls.

And on the larger part.

OPTIMIZED

function showHideErrorBox(theFriendlyName, check){
     var topLevelDiv,
         numDivsAfter;

        if(check === 0){
        // REMOVE THE OLD ERROR MESSAGE IN THE ERROR BOX
            $('.form-validation-errors').find('div').each(function() {
                var $this = $(this);
                if( $this.html().indexOf(theFriendlyName) !== -1 ){
                    $this.remove();
                }
            });
        }

        //  CHECK WHETHER THE ENTIRE MESSAGE BOX CAN BE REMOVED NOW
        topLevelDiv = $(document.getElementById('alert-form-errors'));
        numDivsAfter = topLevelDiv[0].getElementsByTagName('div').length;

        if(check === 0 && numDivsAfter <= 1) {
            return topLevelDiv.hide();
        }

        if(check === 1 && numDivsAfter >= 1) {
            return topLevelDiv.show();
        }    
}

You have also saved on 3 Function calls here too;

Just remember that $ is a function and it's better to call a function once than twice right?

if you ever have something like this

$(this).show();
$(this).val('hello');
$(this).hide();

It should be like this

var $this = $(this);
$this.show()
this.value = 'hello';
$this.hide();

Answer 2

function showHideErrorBox(theFriendlyName, check){
    //remove the old error message in the error box
    if(check === 0){
        $('.form-validation-errors').find('div').each(function(){
            if( $(this).html().indexOf(theFriendlyName) != -1 ){
                $(this).remove();
            }
        });
    }

    //check whether the entire message box can be removed now
    var alertErrors = $('#alert-form-errors');
    var numDivsAfter = alertErrors.find('div').length();
    if(check === 0 && numDivsAfter <= 1){
        alertErrors.hide();
    } else if(check === 1 && numDivsAfter >= 1){
        alertErrors.show();
    }
}

1. There's no need to declare the topLevelDiv and numDivsAfter variables all the way at the top of the function. It's good practice to not declare variables until you actually use them.
2. Don't use caps excessively in your comments. Caps should be used rarely, for emphasis.
3. You should be able to calculate the value for numDivsAfter by using the same find() method you used at the top of the function. Then, instead of calling each(), call length() to get a count of the number of div elements. However, I'm not that well versed in jQuery, so I might be wrong.
4. Assign $('#alert-form-errors') to a variable so you don't have to repeat its ID.
5. You can combine the if statements at the bottom of the function for greater clarity.

Answer 3

If your intent is to hide an error box, if there are no errors inside: you can do that with css alone.

div:empty {
    display:none;
}

See http://jsfiddle.net/bjelline/D2P3r/