10

This is my Ajax:

$("form[0] :text").live("keyup", function(event) {

    event.preventDefault();
    $('.result').remove();
    var serchval = $("form[0] :text").val();

    if(serchval){

        $.ajax({

            type: "POST",
            url: "<?= site_url('pages/ajax_search') ?>",
            data: {company : serchval},
            success: function(data) {

                var results = (JSON.parse(data));
                console.log(results);

                if(results[0]){
                    $.each(results, function(index) {
                        console.log(results[index].name);
                        $("#sresults").append("<div class='result'>" + results[index].name + "</div>");
                    });
                }
                else {
                    $("#sresults").append("<div class='result'>לא נמצאו חברות</div>");
                }
            }
        });
    }
});

When I type slowly (slower then a letter per second) I get the results correct, when I type faster I get 2 times the same results

example:
slow typing: res1 res2 res3
fast typing: res1 res2 res3 res1 res2 res3

Also, any advice on improving the code would be welcome!

ilyo
  • 35,851
  • 46
  • 106
  • 159
  • You should not call .append() in a loop. Create a string and call .append() once, for performance reasons. Everytime you call .append(), the HTML parser has to work and the browser has to do a partitial redraw. It is faster to do this once with a big chunk than to do this very often with smaller chunks. – ckruse Jul 13 '11 at 12:03
  • @ckruse thanx, I switched to $.each and still the same problem, although now I can type a bit faster not to have it... :) – ilyo Jul 13 '11 at 12:21

2 Answers2

9

Thats what is happening (pseudocode):

When you're typing slow:

.keyup1
.remove1
//asynchronous ajax1 request takes some time here...
.append1
.keyup2
.remove2
//asynchronous ajax2 request takes some time here...
.append2

When you're typing fast:

.keyup1
.remove1
//asynchronous ajax1 request takes some time here...
//and keyup2 happens before ajax1 is complete
.keyup2
.remove2
.append1
//asynchronous ajax2 request takes some time here...
.append2
//two results were appended _in a row_ - therefore duplicates

To solve duplicates problem, you would want to make your results removing/appending an atomic operation - using .replaceWith.

Build results HTML block first as string and then do the .replaceWith instead of .remove/.append:

var result = '';
for (i in results) {
    result += "<div class='result'>" + results[i].name + "</div>";
}

$("#sresults").replaceWith('<div id="sresults">' + result + '</div>');

Another problem (not related to duplicates) may be that older result overwrites newer which arrived earlier (because AJAX is asynchronous and server may issue responses not in the same order it receives requests).

One approach to avoid this is attaching roundtrip marker (kind of "serial number") to each request, and checking it in response:

//this is global counter, you should initialize it on page load, global scope
//it contains latest request "serial number"
var latestRequestNumber = 0;

$.ajax({
    type: "POST",
    url: "<?= site_url('pages/ajax_search') ?>",
    //now we're incrementing latestRequestNumber and sending it along with request
    data: {company : serchval, requestNumber: ++latestRequestNumber},
    success: function(data) {
        var results = (JSON.parse(data));
        //server should've put "serial number" from our request to the response (see PHP example below)
        //if response is not latest (i.e. other requests were issued already) - drop it
        if (results.requestNumber < latestRequestNumber) return;
        // ... otherwise, display results from this response ...
    }
});

On server side:

function ajax_search() {
    $response = array();

    //... fill your response with searh results here ...

    //and copy request "serial number" into it
    $response['requestNumber'] = $_REQUEST['requestNumber'];

    echo json_encode($response);
}

Another approach would be to make .ajax() requests synchronous, setting async option to false. However this may temporarily lock the browser while request is active (see docs)

And also you should definitely introduce timeout as algiecas suggests to reduce load on server (this is third issue, not related to duplicates nor to request/response order).

lxa
  • 3,234
  • 2
  • 30
  • 31
  • Why is there a difference between `replace` and `remove/append`? Also, do you have any working example of a roundtrip marker? I don't fully understand how it works. – ilyo Jul 13 '11 at 12:51
  • because 2 `remove`/`append` blocks with tight timing can result in `remove1remove2`/`append1append2` (which causes your problems with duplicates), while `replaceWith` atomically switches DOM node, so you would get either result1 or result1, but not both – lxa Jul 13 '11 at 12:57
  • Regarding roundtrip marker: i'll add some comments into answer in a few minutes. – lxa Jul 13 '11 at 13:02
  • Wow! thanx for the awesomely detailed answer! – ilyo Jul 13 '11 at 13:12
  • last question :) what does `return;` do? cancels the ajax operation? – ilyo Jul 14 '11 at 13:26
  • `return` exits current function. To cancel ajax request you'd have to use special method, [see this](http://stackoverflow.com/questions/446594/kill-ajax-requests-using-javascript-using-jquery/446626#446626). – lxa Jul 14 '11 at 13:42
1

You should involve some timeout before calling ajax. Something like this should work:

var timeoutID;

$("form[0] :text").live("keyup", function(event) {

    clearTimeout(timeoutID);

    timeoutID = setTimeout(function()
    {
       $('.result').remove();
       var serchval = $("form[0] :text").val();

       if(serchval){

           $.ajax({

               type: "POST",
               url: "<?= site_url('pages/ajax_search') ?>",
               data: {company : serchval},
               success: function(data) {

                   var results = (JSON.parse(data));
                   console.log(results);

                   for (i in results)
                   {
                       console.log(results[i].id);
                       $("#sresults").append("<div class='result'>" + results[i].name + "</div>");
                   }
               }
           });
       }
   }, 1000); //timeout in miliseconds
});

I hope this helps.

algiecas
  • 2,108
  • 14
  • 13