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

Highlight input if empty

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

Problem

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).

Solution

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.

Code Snippets

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. });

Context

StackExchange Code Review Q#8212, answer score: 3

Revisions (0)

No revisions yet.