Function within loop problem

JavaScript
1

Hello, I have a script (got it from codrops for a thumbnail gallery) wherein a function is in a while loop. JSLint tells me this is not a good practise, so I would like to place it outside of the loop or write it in another way. This is the code:


            $('<img  desc="' + thumb.desc + '" alt="' + thumb.alt + '" height="75" />').load(function () {
                    $this = $(this);
                    $tContainer.append($this);
                    count = count + 1;
                    if (count === 1) {
                        loadPhoto($this, "cursorPlus");
                    }
                    if (count === countImages) {
                        $('#thumbsWrapper').empty().append($tContainer);
                        thumbsDim($tContainer);
                        makeScrollable($('#thumbsWrapper'), $tContainer, 15);
                    }
                }).attr('src', thumb.src);

So far I’ve tried rewriting it like this:


                var thbImg = '<img  desc="' + thumb.desc + '" alt="' + thumb.alt + '" height="75" />';
                $this = $(thbImg);
                $tContainer.append($this);
                count = count + 1;
                if (count === 1) {
                    loadPhoto($this, "cursorPlus");
                }
                if (count === countImages) {
                    $('#thumbsWrapper').empty().append($tContainer);
                    thumbsDim($tContainer);
                    makeScrollable($('#thumbsWrapper'), $tContainer, 15);
                }
                $this.attr('src', thumb.src);

This works, but the thumbnails are no longer scrollable, but static. So it seems like the last part of the code doesn’t execute well. Any idea’s how to rewrite this properly? If you need more of the code, i can add it.

2

Hi wwl777,

There are a couple of things you need to watch out for when declaring functions within a loop, which is probably why JSLint is flagging it as a potential problem.

First, there could be a performance hit from re-declaring the function on each iteration. Second, any function that is not executed right away (such as onload callbacks) and uses variables that are incremented/changed within the loop will end up with the values from the last iteration.

If you could post the loop code itself and any addition code from with the loop, we might be able to suggest some changes.

3

Thx for the info. Here is the full function its written in:

http://pastebin.com/fUWxL6T7 (put it on pastebin for syntax highlighting.)

4

OK, so I’ve had a look at your code - there’s no easy way to avoid declaring an anonymous function for each image, but to be honest it should have a negligible affect on performance in this situation.

As for the second potential problem I mentioned, your code already avoided the issue, but I’ve had a go at rewriting the function to reduce the need for some of the extra variables. It also stops JSHint from complaining:

function buildThumbs() {
    // voeg materiaal aan thumbs query toe
    if (typeof materiaal != 'undefined') {
        album += '&materiaal=' + materiaal;
    }
    current = 1;
    $('#imageWrapper').empty();
    $('#loading').show();

    $.getJSON(baseurl + 'thumbs?album=' + album, function (data) {
        var countImages = getDataLength(data),
            $tContainer = $('<div/>', {id: 'thumbsContainer', style: 'visibility:hidden;'});

        data = (countImages === 1) ? [data] : data;

        $.each(data, function(index, thumb) {
            $('<img desc="' + thumb.desc + '" alt="' + thumb.alt + '" height="75" />')
                .attr('src', thumb.src)
                .load(function(){
                    $tContainer.append(this);
                    if (index === 0) {
                        loadPhoto(this, "cursorPlus");
                    }
                    if (index === countImages -1) {
                        $('#thumbsWrapper').empty().append($tContainer);
                        thumbsDim($tContainer);
                        makeScrollable($('#thumbsWrapper'), $tContainer, 15);
                    }
                });
        });
    });
};

A couple of things to mention here: first of all, I added this line:

data = (countImages === 1) ? [data] : data;

to ensure that we always have an array (even if it only contains a single element) that we can loop over using jQuery’s $.each() function.

Secondly, the check to see if materiaal is defined or not was causing a reference error in Chrome - a better way to check is like this:

if (typeof materiaal != 'undefined') {}
5

Thx, I’ll have a look at it.