Search code examples
javascriptjquerybreadcrumbs

javascript nested loop variable scope issue


I am writing a simple breadcrumb inspired by https://stackoverflow.com/a/3834694/721084. How I am trying to achieve this is by classifying each item by the page it would be on. The code below is meant to do that but it always ends up in an infinite loop. What am I doing wrong?

EDIT: Pastebin link to entire JS code http://pastebin.com/nxUhQmqF

Sample DOM:

<ul id="progress_bar" class="nostyle clearfix">
    <li class="first"><a href="">Blah</a></li>
    <li class=""><a href="">Blah</a></li>
    <li class="selected"><a href="">Blah</a></li>
    <li class="last"><a href="">Blah</a></li>
</ul>

JS Code:

    function classifyPages(bcParent, totalItems) {
    var pages       = 1,
        wd          = 0,
        parentWd    = findWidthOfParent(bcParent),
        crumbs      = bcParent.find('li'),
        i           = 0;

    for( i = 0; i < totalItems; i++) {
        wd = 0;
        while(wd < parentWd) {
            crumb = crumbs.eq(i);
            wd += crumb.outerWidth();
            if( wd < parentWd) {
                i += 1;
                crumb.addClass( 'bcPage-' + pages);
            }
        }

        pages += 1;
    }

    return pages;
}

Solution

  • Your i, which is also incremented in the inner loop, will run above totalItems at some time. The non-existing crumb then has a outerWidth of 0 always, and you're caught (as @Oleg V. Volkov described).

    This should work:

    function classifyPages(bcParent, totalItems) {
        var pages       = 1,
            parentWd    = findWidthOfParent(bcParent),
            crumbs      = bcParent.find('li');
    
        for (var i = 0; i < totalItems; i++) {
            for (var wd = 0; wd < parentWd && i < totalItems; ) {
    //                                     ^^^^^^^^^^^^^^^^^
                var crumb = crumbs.eq(i);
                wd += crumb.outerWidth();
                if( wd < parentWd) {
                    i += 1;
                    crumb.addClass( 'bcPage-' + pages);
                }
            }
            pages += 1;
        }
        return pages;
    }
    

    Better:

    function classifyPages(bcParent, totalItems) {
        var pages       = 1,
            parentWd    = findWidthOfParent(bcParent),
            crumbs      = bcParent.find('li'),
            wd          = 0;
    
        for (var i = 0; i < totalItems; i++) {
            var crumb = crumbs.eq(i);
            wd += crumb.outerWidth();
            if( wd >= parentWd) {
                pages += 1;
                wd = 0; // reset
            }
            crumb.addClass( 'bcPage-' + pages);
        }
        return pages;
    }