Improving nested for loops for sub menu creation

Question

I have a nested loop which creates a drop down with 3 levels. Works fine on modern browsers but on IE8 it hangs and takes several seconds when there's even a moderate volume of data.

It creates a list of publishers, when I click a publisher I get that publishers authors, click an author I get the years they're published.

I'm looking for a way of optimizing this to reduce the time it takes to run in IE.

Any ideas?

function createDropDown() {        
        var target = $('#dropdownMenu');
        for (var i = 0; i < data.genres.length; i++) {
                var genreval = data.genres[i].genre;
                for (var j = 0; j < data.genres[i].publishers.length; j++) {
                        var publisherval = data.genres[i].publishers[j].publisher;
                        if (typeof app.cache.publisher[publisherval] == 'undefined') {
                                target.append('<li class="publisher" data-value="' + publisherval + '"><a href="#" class="nextmenu">' + publisherval + '</a><ul class="sub-menu" data-title="publishers"></ul></li>');
                                app.cache.publisher[publisherval] = {
                                        'ul': target.find('>.publisher[data-value="' + publisherval + '"]>.sub-menu'),
                                        'author': {}
                                }; 
                        }
                                var ulauthors = app.cache.publisher[publisherval].ul;
                                for (var k = 0; k < data.genres[i].publishers[j].authors.length; k++) {
                                        var authorval = data.genres[i].publishers[j].authors[k].author + ' (' + genreval + ')';
                                        var author_val = data.genres[i].publishers[j].authors[k].author;
                                        var originalgenre = data.genres[i].publishers[j].authors[k].originalgenre;
                                        if (typeof app.cache.publisher[publisherval].author[authorval] == 'undefined') {
                                                ulauthors.append('<li class="author" data-value="' + authorval + '"><a href="#" class="nextmenu">' + authorval + '</a><ul class="sub-menu" data-title="authors"></ul></li>');
                                                app.cache.publisher[publisherval].author[authorval] = ulauthors.find('>.author[data-value="' + authorval + '"]>.sub-menu');                             
                                        }
                                                                    var ulyears = app.cache.publisher[publisherval].author[authorval];
                                                                    var gItems = "";
                                                                    for (var m = 0; m < data.genres[i].publishers[j].authors[k].authorYears.length; m++) {
                                                                            var yearval = data.genres[i].publishers[j].authors[k].authorYears[m];
                                                                            var year = ulyears.find('.year[data-value="' + yearval + '"]');
                                                                            if (year.size() == 0) {
                                                                                    var id = ++count;
                                                                                    gItems+='<li class="year" data-value="' + yearval + '"><a id="selyear' + id + '" class="addItem" data-id="' + id + '" data-originalgenre ="' + originalgenre + '"data-year="' + yearval + '" data-publisher="' + publisherval + '" data-author="' + author_val '">' + yearval + '</a></li>';
                                                                            }
                                                                    }
                                        ulyears.append(gItems);
                                };
                };
        };
    }

Edited Code which doesnt append the authors:

if (typeof app.cache.publisher[publisherval].author[authorval] == 'undefined') {
app.cache.publisher[publisherval].author[authorval] = $('<li class="author" data-value="' + authorval + '"><a href="#" class="nextmenu">' + authorval + '</a><ul class="sub-menu" data-title="authors"></ul></li>').children('ul').appendTo(ulauthors);
}

Answer 1

Formatting

I found this exceedingly difficult to read. The indentation in particular is inconsistent. I highly suspect that this is because your code contains mixed tabs and spaces. (StackExchange silently converts tabs into four spaces. If this is not the case, please ignore this section.)

I don't want to start a holy war or anything, but there are three main camps here.

1. Indent exclusively with tabs. This is nice when using text editors with a configurable width for tabs. It also can marginally speed up a site, but that's only if you aren't already stripping out whitespace before sending to client. It's also negligible in the era of gzip.

2. Indent exclusively with spaces. This guarantees that the code will look the same for everyone. Since it isn't configurable, though, if you like eight spaces and someone else likes two spaces, they can't just make it happen with a setting; they'll need to do a replace on the file.

3. Mixed: indentation levels use tabs, but alignment is done with spaces. This is the style I personally use.

Aside: Semicolons are typically not necessary after braces. You're using a code block for a for loop, not a function expression.

Naming

• I don't see any point in naming a variable genreval; just genre is enough.
• Convention dictates that jQuery object variables are prefixed with a $.
• I think that it's particularly confusing to have authorval and author_val. If anything, I'd have author_val → author and authorval → authorWithGenre or something... I really don't know how I'd name this. Is it critical that an author's genres are distinguished?
• You have app.cache.publisher[].ul and app.cache.publisher[].author[] both being jQuery objects for a ul. This is inconsistent. I might prefer each element of author[] to be an object with just the key ul, i.e. author[] = { 'ul': ... };.

Performance

It looks like you're appending an element and immediately performing a find for it. This is incredibly wasteful. You can just use appendTo instead.

I can think of two ways; one creates two objects and uses two appendTos, while one performs a children query. I wrote a quick-and-dirty jsPerf. The children query runs the fastest; your original is 91% slower.

// children
app.cache.publisher[publisherval].author[authorval] = {
    'ul': $('<li class="author" data-value="' + authorval + '"><a href="#" class="nextmenu">' + authorval + '</a><ul class="sub-menu" data-title="authors"></ul></li>').children('ul').appendTo(ulauthors)
}
// double append
app.cache.publisher[publisherval].author[authorval] = {
    'ul': $('<ul class="sub-menu" data-title="authors"></ul>').appendTo($('<li class="author" data-value="' + authorval + '"><a href="#" class="nextmenu">' + authorval + '</a></li>').appendTo(ulauthors))
}

Answer 2

There are a few things you can do to improve your JavaScript code.

For example, you can save the element you are using in each loop:

var currentPuiblisher = data.genres[i].publishers[j]

Every time you use a object like data.genres[i].publishers[j], the browser has to walk through every property, slowing down the process.

Your loops are checking the length of the array every iteration. This will be faster:

for (var k = 0, authLength = data.genres[i].publishers[j].authors.length; k < authLength; k++)

A microtemplate library would help you a lot generating the HTML, but anyway it would be better if you generate the full string and append once to the domDOM, there are a lot of append operations that require some time.

In general, I would try to use a dynamic solution: just generating the first level of the menu, and then dynamically add the submenus on hover.