CODEWITHSUNDEEP
jquery
Hussein
Hussein

Reputation: 42818

How to rewrite this function using jquery

I'm trying to write shorter code using jquery. What is the best way to rewrite this function in jquery. 

function searchDone(results) {
    var result = null;
    var parent = document.getElementById('resultList');
    parent.innerHTML = '';
    var child = null;

    for (var i = 0; i < results.SearchResponse.Image.Results.length; i++) {
        result = results.SearchResponse.Image.Results[i];
        child = document.createElement('li');
        child.className = "resultlistitem";

        child.innerHTML = '<a href="' + result.MediaUrl +'"><img src="' +
            result.Thumbnail.Url +'" alt="' + result.Title +'" /></a>';

        parent.appendChild(child);
    }
}

Upvotes: 1

Views: 353

Answers (5)

Michal
Michal

Reputation: 13639

Assuming your results are JSON 

    function searchDone(results) {
     $("#resultList").empty();
     html = "";

//creating this as a string and appending at the end is faster than appending each element 'live' - depends how many search results you have
    $.each( results.SearchResponse.Image.Results, function(key,result){
      html +='<li><a href="' + result.MediaUrl +'"><img src="' +
                result.Thumbnail.Url +'" alt="' + result.Title +'" /></a></li>';
     });
     $("#resultList").append(html);
    }

Upvotes: 1

PetersenDidIt
PetersenDidIt

Reputation: 25620

Its a bad idea to continually append to the same element in the DOM in loop. It means that the DOM will get redrawn every single time you append a new element.

Do something like this to append only once:

function searchDone(results) {
 var list = [];
    $.each(results.SearchResponse.Image.Results, function(i, v) {
        list.push('<li class="resultlistitem"><a href="' + v.MediaUrl +'"><img src="' + v.Thumbnail.Url +'" alt="' + v.Title +'" /></a></li>');
    });
 $('#resultList').empty().append(list.join(''));
}

This creates an array will all the "children" in it. Once the each as looped over all the results it empties out the #resultList and appends all the children.

Upvotes: 1

Šime Vidas
Šime Vidas

Reputation: 185933

function searchDone(results) {
    var r,
        p = $('#resultList').empty(),
        arr = results.SearchResponse.Image.Results;

    for (var i = 0; i < arr.length; i++) {
        r = arr[i];
        p.append('<li class="resultlistitem"><a href="' + r.MediaUrl + '"><img src="' + r.Thumbnail.Url + '" alt="' + r.Title + '" /></a><li>');
    }   
}

As @Petersen already pointed out, DOM manipulation should be avoided inside loops. The alternative code below will create the HTML source code string via the loop, and the append the whole string at once.

function searchDone(results) {
    var r,
        p = $('#resultList').empty(),
        arr = results.SearchResponse.Image.Results,
        s = '';

    for (var i = 0; i < arr.length; i++) {
        r = arr[i];
        s += '<li class="resultlistitem"><a href="' + r.MediaUrl + '"><img src="' + r.Thumbnail.Url + '" alt="' + r.Title + '" /></a><li>';
    }

    p.append(s);
}

Upvotes: 1

Luca Filosofi
Luca Filosofi

Reputation: 31173

If you dont care about effects in the sense of "display one message one by one" you can display it all together... like this

function searchDone(results) {
    var lis = [];
    $.each(results.SearchResponse.Image.Results, function(i,item){
    lis.push('<li class="resultlistitem"><a href="' + item.MediaUrl +'"><img src="' +
            item.Thumbnail.Url +'" alt="' + item.Title +'" /></a></li>');
    }); $('#resultList').html( lis.join('') );
}

otherwise with a little bit of code you can have a nice fadeOut effect like this:

DEMO: http://jsbin.com/odimi4

function searchDone(results) {
    $.each(results.SearchResponse.Image.Results, function(i,item) ) {
        $('<li class="resultlistitem">').html('<a href="' + item.MediaUrl +'">'+
          '<img src="' +item.Thumbnail.Url +'" alt="' + item.Title +'" /></a>')
          .appendTo('#resultList').delay(i+'000').fadeOut(200,function(){$(this).remove()});
    }); 
}

Upvotes: 1

Jared Farrish
Jared Farrish

Reputation: 49208

function searchDone(results) {
    var result = null;
    var item = '';
    $('#resultList').empty();
    for (var i = 0; i < results.SearchResponse.Image.Results.length; i++) {
        result = results.SearchResponse.Image.Results[i];
        item = '<li>' +
                 '<a href="' + result.MediaUrl + '"> ' +
                   '<img src="' + result.Thumbnail.Url + '" alt="' + result.Title + '"/>' + 
                 '</a>' +
               '</li>';
        $('#resultList').append(item);
   }
}

Upvotes: 1

Related Questions