Infinite loop when using generic solution to split up carousel contents

Question

Basically, I'm getting an infinite loop and maybe I'm working too hard but I can't see why.

Context:

I'm using a carousel (Bootstrap's). The contents of the carousel is generated and pushed into one carousel slide, then the goal is to take the contents and split it up into multiple slides if the number of items inside surpass a certain pre-defined max-length property (5). I got this working fine for a specific use case of the carousel (a table being spread across the multiple slides if there are more than 5 table rows), but it's not generic enough. What happened is that the JS would take the overflown table rows (i.e. of index 5 and up), create a new slide from a harcoded HTML string in the function (a slide div containing all the markup for the table yet empty) and push those extra rows into it.

To make it more generic, I've decided to use classes like carousel_common_list and carousel_common_item which would be applied to the tbody and trs in the case I've explained. Then, I've to handle the template in a decoupled way. What I've tried to do is, take a clone of the original sole slide, empty the carousel_common_list and push any overflown carousel_common_items into it, and so on. But I get an infinite loop.

Code

What I've called a slide so far is called an item in the code (to match Bootstrap's carousel's item class for slides).

var carousels = $('div.carousel'),
    carouselCommonListClass = 'carousel_common_list',
    carouselCommonItemClass = 'carousel_common_item',
    items_per_slide = 5;

$.each(carousels, function (index, element) {//for each carousel

    var $carousel = carousels.eq(index),
        $items = $carousel.find('.item');

    var getItemTemplate = function ($item) {
        $copy = $item.clone();//take the html, create a new element from it (not added to DOM)
        $copy.find('.' + carouselCommonListClass).empty();
        $copy.removeClass('active');
        return $copy;
    }


    var splitUpItem = function ($item, $itemTemplate) {
        var $bigList = $item.find('.' + carouselCommonListClass), group;
        while ((group = $bigList.find('.' + carouselCommonItemClass + ':gt(' + (items_per_slide - 1 ) + ')').remove()).length) {
            var $newItem = $itemTemplate;
            $newItem.find('.' + carouselCommonListClass).prepend(group);
            $newItem.insertAfter($item);
            splitUpItem($newItem, $itemTemplate);//infintely called
        }
    }

    //foreach item
    $.each($items, function (item_index, item_element) {//for each slide, in each carousel

        var $item = $items.eq(item_index);

        splitUpItem($item, getItemTemplate($item));
    });
});

FYI, this works like expected when the line marked with //infintely called is commented out; i.e. splits one oversized slide into one slide of items_per_slide length and another slide (which could be over items_per_slide in length if the original sole slide was over items_per_slide * 2 in length.

Also, I took this answer and modified it for the contents of splitUpItem().

Note:

I know it's not the most usable or accessible solution to split tables, lists, etc. over multiple slides like I am, but if you've a better idea answer my open question on that.

score 0 · Answer 1 · answered Sep 28 '12 at 14:50

0

You're not getting an infinite loop per se, in that you're not infinitely stuck in the same while loop. As you mention, when you remove the //infinitely called line you're fine. The first pass through that while loop, the length you compute will equal the number of items (with gt:(4)) in all the lists in $item. You then remove all those items, so the next pass through will have that number equal to 0. This will always be the behaviour of that loop, so it really doesn't need to be a loop, but that's not the main problem.

The problem is that it's a recursive call. And the only guard you have against making the recursive call infinite is the condition in your while loop, but that condition will always be met the first pass through. In fact, if $item has 5 lists, each with 3 items with gt:(4), then $newItem will have 5 lists, each with 5 x 3 = 15 items. So when splitUpItem gets called on $newItem, the condition in your while loop will again first be non-zero. And then it'll get called again, and that number will be 5 x 15 = 75. And so on. In other words, you're recursively calling this function, and your guard against this call being made infinitely many times is to check that some number is 0, but the number there will actually grow exponentially with each recursive call of splitUpItem.

Hope that answers your question about why it's "infinitely looping." Gotta get to work, but I'll try to suggest a better way to split up the slides tomorrow if no one else has by then.

answered Sep 28 '12 at 14:50

Amit Kumar Gupta

17,184
7
46
64

Thanks for the answer. `it really doesn't need to be a loop` is wrong I think . I'm thinking I need to update `$bigList` instead of calling `splitUpItem` again. (This still doesn't fix it though) On the rest of your answer, I'm not exactly following. I understand that you're saying it accumulates but I don't see how. It re-sets `group` to the items with `gt(4)` each time right? And `$bligList` is based off `$newItem` so the list should be getting smaller. – Adam Lynch Sep 28 '12 at 16:14
Can you explain a little more? Maybe you can tell where my misunderstanding is, from my last comment. – Adam Lynch Sep 28 '12 at 16:22
1

When I said it doesn't need to be a loop, I meant that since you remove all the `gt(4)` items under `$bigList` in the first pass through the loop, and you don't change `$bigList` inside the loop, the next pass to the top of the loop (if there ever were one) would necessarily not enter. As such your `while` is basically just behaving like an `if`. This was perhaps a moot point because your code never even reaches the top of the loop again, because it gets caught in an infinite recursion before it has a chance. – Amit Kumar Gupta Sep 29 '12 at 22:10
If `$item` has n `.list` children, each of which has m `.item` children, then `var group = $item.find('.list).find('.item')` will be an array of nm items. If you then take `$newItem` which has n `.list` children, each of which has 0 `.item` children, and do a `$newItem.find('.list').prepend(group)` then you will prepend all mn elements of `group` to each of the n `.list` children of `$newItem`, giving `$newItem` n²m grandchild `.item` elements. – Amit Kumar Gupta Sep 29 '12 at 22:18
But `n` is always 1. What I think should happen is the following (ignoring `n` since it's always 1, and assuming I've now updated `$bigList` at the bottom of the `while` instead of a call to `splitUpItem`). Let's call `ìtems_per_slide` `i`. The first time, `group` contains `m - i` items, this is prepended to a new list, which is now `$bigList` for the next iteration. Then for the second iteration,`group` contains `(m - i) - i` items, and so on until `group` contains `0` items and it *should* stop. – Adam Lynch Sep 30 '12 at 10:55
My apologies, I misinterpreted what `:gt` was doing. So given that n=1 and you've replaced the `//infinitely called` line with `$bigList = $newItem.find('.'+carouselCommonListClass)`, the problem now is (and in fact, this was probably the real problem before) that `$itemTemplate` is not the hollow template you think it is. Once you do `var $newItem = $itemTemplate; $newItem.find('foo').prepend('bar');` you can `alert` out `$itemTemplate.html()` and you'll see `'bar'` in there. – Amit Kumar Gupta Sep 30 '12 at 19:06
I understand what you're saying about `prepend` but is the jsfiddle your fix? Why did you change it to use `:lt` instead of `:gt`? I need to be in the original order. But that's ok. The jsfiddle doesn't do what I expect with `.item`s / carousel slides. See [this](http://jsfiddle.net/eC4e5/2/); the end result in five `.item`s (which is right), but one contains four items (which is right), but one contains fourteen and the remaining three `.item`s (which are in between those two) are empty – Adam Lynch Oct 01 '12 at 06:51
Ok, I've fixed it (almost). See this [new jsfiddle](http://jsfiddle.net/JnjE6/1/). I was inserting after `$item` instead of the latest one (which would mess the order up), so I'm updating an `$oldItem` variable now, and I have to regenerate the template each time *inside* `splitUpItem`. If you can fix that (I want to be able to take in the template as a param like before), then update your answer and I'll accept. – Adam Lynch Oct 01 '12 at 07:20
**Ok, I've fixed it**. It's ok to take the item template as a param but the line where it's used needs to be changed to: `var $newItem = $itemTemplate.clone();` :) – Adam Lynch Oct 01 '12 at 08:43

Infinite loop when using generic solution to split up carousel contents

Code

1 Answers1