Improving search through XML file

Question

I'm working on a function that extracts information from an XML document and compares to what the user has typed in. Can you suggest any ways in which I can improve it? The variables in the XML file are compared against txtSearch.

function checkXML() {
    var s = "";
    var albumPages = [];

    for (var i = 0; i < xmlalbums.length; i++) {
        var xmlalbum = xmlalbums[i];
        var album_title = dataFromTag(xmlalbum, 'title');
        var artist = dataFromTag(xmlalbum, 'artist');
        var xmltracks = xmlalbum.getElementsByTagName('track');
        for (var t = 0; t < xmltracks.length; t++) {
            var track = xmltracks[t];
            song_title = dataFromTag(track, 'title')
            albumPages[albumPages.length] = artist.toUpperCase() + ':' + album_title.toUpperCase() + ':' + song_title.toUpperCase();
        }
    }

    var resultList = '';
    var resultCount = [];

    for (var i = 0; i < albumPages.length; i++) {
        var searchArtist = albumPages[i].substring(0, albumPages[i].indexOf(":"));
        var searchAlbum = albumPages[i].substring(albumPages[i].indexOf(":") + 1, albumPages[i].lastIndexOf(":"));
        var searchSong = albumPages[i].substring(albumPages[i].lastIndexOf(":") + 1, albumPages[i].length);
        if (searchArtist.indexOf(txtSearch) != -1 || searchAlbum.indexOf(txtSearch) != -1 || searchSong.indexOf(txtSearch) != -1) {
            resultList += "<option>" + searchSong + "</option>"
            resultCount[resultCount.length] = i;
        }
    }

    if (resultList.length == 0) {
        s += "<span style='font-size: 2em;color: #fff;font-family: Arial;'>No result found!</span>"
    }
    else if (resultList.length > 0) {
        s += '<select size="' + (resultCount.length + 1) + '" onclick="parent.col3(value);">';
        s += resultList;
        s += '</select>';
    }
    else {
        s += "<span style='font-size: 2em;color: #fff;font-family: Arial;'>Awaiting search query...</span>"
    }

    with(document.getElementById('col2').contentDocument) {
        open();
        write(s);
        close();
    }
}

Answer 1

A few general things from a quick scan:

• declare all your variables at the top in one var statement rather than using it throughout the script
• use somearray.push(x) rather than somearray[somearray.length]=x, as its shorter and means you don't have to look up the .length property every time in your loop
• in general, avoid with; there are many reasons why e.g. http://www.yuiblog.com/blog/2006/04/11/with-statement-considered-harmful/
• string concatenation is slow; instead of many += operators, create an array, use .push() as above to add strings to the array, then use .join('') to merge them together.

And yes, XPath would be a better approach to extract data from XML.

Answer 2

I'd create a local variable for albumPages[i] in the second for loop:

for (var i = 0; i < albumPages.length; i++) {
    var albumPage = albumPages[i];
    var searchArtist = albumPage.substring(0, albumPage.indexOf(":"));
    ...
}

It would remove a lot of unnecessary indexing and make the code easier to read.

But... in the first for the code concatenates the artist, album title and song title strings and in the second loop it parses it with string functions. You should not do that, it looks completely unnecessary. Use a better data structure (class, struct, array etc. - I don't know JavaScript so well), store your data into it in the first loop and iterate over it in the second loop without parsing.

Answer 3

How exactly do you want it improved?

• better performance?

• better relevance and recall of the search results?

• better code maintainability?

• better robustness / security?

One obvious criticism is that generating XML/HTML output using string concatenation is both (a) inefficient, and (b) error-prone (for example, there is no attempt to escape special characters or detect special sequences like "]]>")

Answer 4

I have not benchmarked the different processes, but it is my belief that using XPath to traverse/parse your XML would be a much better option than using string methods.

It's a big subject, but you might want to start here