SitePoint Sponsor

User Tag List

Results 1 to 5 of 5
  1. #1
    SitePoint Member wwl777's Avatar
    Join Date
    Aug 2011
    Posts
    17
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    function within loop problem

    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:


    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:

    Code:
                    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. #2
    Community Advisor bronze trophy
    fretburner's Avatar
    Join Date
    Apr 2013
    Location
    Brazil
    Posts
    1,436
    Mentioned
    45 Post(s)
    Tagged
    13 Thread(s)
    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. #3
    SitePoint Member wwl777's Avatar
    Join Date
    Aug 2011
    Posts
    17
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Thx for the info. Here is the full function its written in:

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

  4. #4
    Community Advisor bronze trophy
    fretburner's Avatar
    Join Date
    Apr 2013
    Location
    Brazil
    Posts
    1,436
    Mentioned
    45 Post(s)
    Tagged
    13 Thread(s)
    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:

    Code JavaScript:
    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:
    Code JavaScript:
    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:
    Code JavaScript:
    if (typeof materiaal != 'undefined') {}

  5. #5
    SitePoint Member wwl777's Avatar
    Join Date
    Aug 2011
    Posts
    17
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Thumbs up

    Thx, I'll have a look at it.


Tags for this Thread

Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •