Highlight input if empty

Question

This snippets works, but I think it can be optimized:

/* highlight input, if it is empty */
$(function() {
    var highlight_if_no_val=function () {
        $(this).toggleClass('highlight-input', !$(this).val());
    }
    $('#id_index-EditForm-gpnr').keyup(highlight_if_no_val).each(highlight_if_no_val);
}
);

Is there a simpler solution?

The .each(hightlight_...) is done to set the highlight after the page loaded (it can be initially empty or initially filled).

Answer 1

Your code looks good enough. "optimize" is a vague action.

1. /* highlight input, if it is empty */
2. jQuery(function ($) {
3.     function toggleHighlightIfEmpty() {
4.         var $this;
5.         $this = $(this);
6.         $this.toggleClass('highlight-input', !$this.val());
7.     }
8.     $('#id_index-EditForm-gpnr').on('keyup.highlight', toggleHighlightIfEmpty).trigger('keyup.highlight');
9. });

Line 2:

I'd use the aliasing form of document.ready to prevent conflicts with other libs

Line 3:

I'd use the function declaration form over variable declaration form for the internal function. It's really a personal preference sort-of-thing. Additionally, using camelCase is relatively standard for JavaScript function names.

Lines 4-6:

I'd store a reference to $this so that you don't keep calling the jQuery factory function. It shouldn't be significant in any way for performance, but it can save some headaches with perens which can be easily misplaced

Line 8:

I'd set the event using a namespaced event name so that it can be triggered and removed simply by calling $(...).off('keyup.highlight'). Instead of passing the function to each again, I prefer to use trigger. If you need backwards compatibility, use bind instead of on.

None of these "optimizations" should lead to code that's in any significant way different as far as performance is concerned. It really comes down to personal preference.

Answer 2

Just a thought ... with explination:

//  Move your variable function outside of your ready call to make it accessible later
//  if needed, for instance in another script call from a dynamically drawn in view,
//  Alsot, shorten the name so as to save milliseconds on call time, and minutes in
//  coding it (possibly repeatedly) later.
//  I left the name descriptive, but you could really shortin even more
function zero_highlight() {
    $(this).toggleClass('highlight-input', !$(this).val());
}
//  Now begin the ready code. FYI, for others looking at this, you might should have
//  mentioned you were using jQuery (though looking at it with an experienced eye,
//  it appears obvious)
$(function() {
    //  Here again you might really want to consider a name change, call change,
    //  especially in using .each as (i found in personal experience) jquery's 
    //  .each funtion tends to work much more functionally when calling a tagname
    //  or classname as opposed to name multiple items with the same ID
    //  Not to mention, having multiple items with the same ID could be devastating
    //  to your dynamic functionality in later code.
    $('.frm-edit-gpnr').keyup(zero_highlight).each(zero_highlight);
});