Having my else statement executed even if condition isn't met in jQuery

Question

Well, my title can't be true, since a programming language can't make such a mistake, but it's happening. So we can safely say there is a problem in my logic.

I have a piece of jQuery/JS code that checks for the number of characters "to go" just like here in SO's comment section before an AJAX call is made. So the code is thought like this:

If the number of characters in the input is less then N count the characters down (eg., 2 chars to go, 1 char to go...)
Else make an AJAX call and display the result on success.

When you go to the form for the first time, this process goes smooth:

• You get the empty field
• Start typing
• The character indicator keeps decreasing
• We hit the "breakpoint" where it stops checking
• AJAX call is made
• We get a message displayed on the success callback

But when I keep removing the characters with backspace (or delete) and I go again under the breakpoint (where the "N to go" must be seen) I get the "N to go" part displayed again, but the AJAX call goes through too even though it's wrapped in an else! This is pretty weird. The result looks like this:

Where the red text is the AJAX success callback and the yellow part is the character counter.

This is my jQuery:

var delay = (function(){
    var timer = 0;
        return function(callback, ms){
            clearTimeout (timer);
            timer = setTimeout(callback, ms);
        };
})();

$('#reg_email').keyup(function(){

    var min_password_chars = 6;
    var email_input = $('#reg_email').val();
    var email_prev  = '';

    if (email_input.length < min_password_chars)
    {
        $('.email-check').text('');

        if ($('.email-check').children('.yellowish').length == 0)
        {
            $('.email-check').append('<span class="yellowish"><span class="counter">6</span> to go</span>');
        }

        $('.email-check .counter').text(min_password_chars - email_input.length);
    }
    else
    {
        delay(function(){
            $.ajax({
                type: 'GET',
                url: 'ajax.php?action=check_email',
                data: { email: JSON.stringify(email_input) },
                success: function(data){
                    var response = $.parseJSON(data);                           
                    $('.email-check').append(response.message);

                    email_prev += response.sent_email;
                }
            });
        }, 1500);

        if (email_input != email_prev)
        {
            $('.email-check').text('');
        }
    }
});

And this is the HTML:

<div class="form-group">
    <label for="rep_email">Repeat email</label>
    <input type="email" class="form-control" id="rep_email" name="email_rep" placeholder="Enter your email" required>
    <p class="email-check pull-right validate-note"><span class="yellowish"><span class="counter">6</span> to go</span></p>
</div>

What could I possibly be doing wrong? Where is my logic failing me?

Answer 1

It looks like a race condition, compounded by $.ajax and delay(). Your ajax call will not fire until the next tick of the event loop, and so the message that your ajax returns will be appended after you exit the if block.

I'd do the following:

1. Debounce the event handler (only call the event handler if it's the last event within a given time window). Try using underscore.js's debounce, it's pretty good for this situation.
2. Move the helper text clear function to the top of the event handler (don't need to duplicate that code)

Here's how I would re-write it:

$('#reg_email').keyup(_.debounce(function() {

    $('.email-check').text('');

    var min_password_chars = 6;
    var email_input = $('#reg_email').val();
    var email_prev  = '';

    if (email_input.length < min_password_chars) {
        if ($('.email-check').children('.yellowish').length == 0) {
            $('.email-check').append('<span class="yellowish"><span class="counter">6</span> to go</span>');
        }

        $('.email-check .counter').text(min_password_chars - email_input.length);
    } else {
        $.ajax({
            type: 'GET',
            url: 'ajax.php?action=check_email',
            data: { email: JSON.stringify(email_input) },
            success: function(data){
                var response = $.parseJSON(data);                           
                $('.email-check').append(response.message);

                email_prev += response.sent_email;
            }
        });
    }  
}), 1500);  

Answer 2

Your ajax calls are doubly asynchronous so that is probably what is confusing your debugging and the result. This type of thing is actually hard to debug with breakpoints. I find I often have to put console.log() statements in the code so I can see the actual sequence of events in real time.

The operative part of the else clause executes sometime later after the else clause is first encountered and it's doubly delayed because of both your delay() and the asynchronous nature of AJAX calls. That makes it hard to debug and can easily lead to programming errors if you aren't coding for that specific situation.

Firstly they're asynchronous because you're using a delay() which I assume is a setTimeout(). That means you don't even start the ajax call until sometime later. Second, they're asynchronous because they're async ajax calls. So, they call the success handler in sometime AFTER the else branch executes. In fact, you could have typed other characters before they finish executing.

You also have closure issues where you may be using data in your success handler that isn't the same as when the ajax call was originally sent (because it has changed since the ajax call stated).

---

In the code below, I applied these fixes:

1. Cancels any previous timer that has not yet fired so you don't get timers going and going when more characters have been typed. This will prevent the sending of an ajax call when you're about to schedule another one.
2. Always sends the latest value to the server, even if it has been modified since the timer was initially set.
3. If it has been modified since this timer was set and is now too short, don't send that to the server as that too short error will have already been reported.

What this does not do is to cancel any inflight ajax call when you schedule the next ajax call, but that's just a server efficiency thing, not a correctness thing. Your code also assumes that multiple ajax calls in flight at the same time will be processed and returned in order. That is usually the case in the practical world, but isn't guaranteed without coding to specifically order them.

var emailTimeout;
$('#reg_email').keyup(function(){
    var min_password_chars = 6;
    var email_input = $('#reg_email').val();
    var email_prev  = '';

    if (email_input.length < min_password_chars)
    {
        $('.email-check').text('');

        if ($('.email-check').children('.yellowish').length == 0)
        {
            $('.email-check').append('<span class="yellowish"><span class="counter">6</span> to go</span>');
        }

        $('.email-check .counter').text(min_password_chars - email_input.length);
    }
    else
    {
        clearTimeout(emailTimeout);
        emailTimeout = setTimeout(function(){
            // get latest email value, might have changed
            var latest_email_input = $('#reg_email').val();
            // if it's not long enough, don't send to the server
            if (latest_email_input.length < min_password_chars) return;

            $.ajax({
                type: 'GET',
                url: 'ajax.php?action=check_email',
                // always retrieve latest value of data (may have changed)
                data: { email: JSON.stringify(latest_email_input) },   
                success: function(data){
                    var response = $.parseJSON(data);                           
                    $('.email-check').append(response.message);

                    email_prev += response.sent_email;
                }
            });
        }, 1500);

        if (email_input != email_prev)
        {
            $('.email-check').text('');
        }
    }
});

---

FYI, here's a little more versatile version of your cool delay function that can be used for multiple different uses at the same time by just passing a different tag string for different uses:

var delay = (function(){
    var timers = {};
        return function(tag, callback, ms){
            clearTimeout(timers[tag]);
            timers[tag] = setTimeout(function() {
                delete timers[tag];
                callback();
            }, ms);
        };
})();

// sample usage
delay("emailKey", fn, 1500);
delay("search", fn, 500);

Answer 3

Say you enter abcdefgh in the email field and then backspace over it, this will cause 5 ajax calls, spread out over at least 5*1.5=7.5 seconds due to the delay.

To count this, start at abcde:

• abcde - nothing yet.
• abcdef - ajax call 1
• abcdefg - ajax call 2
• abcdefgh - ajax call 3
• abcdefg - ajax call 4
• abcdef - ajax call 5

The ajax calls will continue after editing the field, with at least 1.5 seconds in between them.

Moreover, if you are using breakpoints, then this will pause the delays when you pause the program. When you run it again, it will continue where it left off, with queued ajax events, making it appear like they are running on their own, but are actually left over from the last run.