admin管理员组

文章数量:1026959

Currently I have a loop that updates the DOM in each iteration; I have learned this is a bad practice & you should update the DOM as little as possible for better speed.

So I was wondering how I go about editing the below so I can store all the elements in one element or something & then do a single DOM addition once the loop ends.

Here is the loop..

    for (var i = spot; i < spot + batchSize && i < cats.options.length; i++) {

        // Check if the cat is selected

        if (cats.options[i].selected == true) {

            // Set this category's values to some variables
            var cat_id = cats.options[i].getAttribute('value');
            var cat_name = cats.options[i].text;     

            if (checkCatSICAdd(cat_id) === false) {           

                // Now we create the new element
                var new_option = document.createElement('option');

                // Add attribute
                new_option.setAttribute('value',cat_id);

                // Create text node
                var new_text_node = document.createTextNode(cat_name);

                // Append new text node to new option element we created
                new_option.appendChild(new_text_node);

                // Append new option tag to select list
                sel_cats.appendChild(new_option);

            } else {
                failed++;
            }

        }

    }

Currently I have a loop that updates the DOM in each iteration; I have learned this is a bad practice & you should update the DOM as little as possible for better speed.

So I was wondering how I go about editing the below so I can store all the elements in one element or something & then do a single DOM addition once the loop ends.

Here is the loop..

    for (var i = spot; i < spot + batchSize && i < cats.options.length; i++) {

        // Check if the cat is selected

        if (cats.options[i].selected == true) {

            // Set this category's values to some variables
            var cat_id = cats.options[i].getAttribute('value');
            var cat_name = cats.options[i].text;     

            if (checkCatSICAdd(cat_id) === false) {           

                // Now we create the new element
                var new_option = document.createElement('option');

                // Add attribute
                new_option.setAttribute('value',cat_id);

                // Create text node
                var new_text_node = document.createTextNode(cat_name);

                // Append new text node to new option element we created
                new_option.appendChild(new_text_node);

                // Append new option tag to select list
                sel_cats.appendChild(new_option);

            } else {
                failed++;
            }

        }

    }
Share Improve this question edited Aug 28, 2012 at 19:18 GEOCHET 21.3k15 gold badges77 silver badges99 bronze badges asked Apr 4, 2011 at 21:30 BrettBrett 20.2k57 gold badges166 silver badges304 bronze badges
Add a ment  | 

5 Answers 5

Reset to default 3

Working with DOM element in the loop is slow - no matter if you attach them to the document or not. Attaching them at the end is a bit faster since only only redraw is required but it's still slow.

The proper way is generating a plain old string containing HTML and attaching this string to the DOM using the innerHTML property of a DOM element.

Your code should be ok. The DOM won't actually redraw until the Javascript has finished executing. However, if you've encountered a problem browser that does perform badly, you could try creating a new select before your loop that is not yet attached to the DOM, populating it as you are now, then replacing sel_cats with that new select at the end. That way, the DOM is only updated once.

Your way is good enough unless you have great many items added to sel_cats - you add to the DOM only once.

The only way to improve efficiency might be to store the options as raw HTML then assign that after the loop:

var arrHTML = [];
for (var i = spot; i < spot + batchSize && i < cats.options.length; i++) {
    // Check if the cat is selected
    if (cats.options[i].selected == true) {
        // Set this category's values to some variables
        var cat_id = cats.options[i].value;
        var cat_name = cats.options[i].text;     
        if (checkCatSICAdd(cat_id) === false) {           
            arrHTML.push("<option value=\"" + cat_id + "\">" + cat_name + "</option>";
        }
        else {
            failed++;
        }
    }
}
sel_cats.innerHTML = arrHTML.join("");

Once you have the select list assigned to a variable, remove it from the dom using removeChild on its parent tag. You can then use appendChild in the loop before adding the select list back into the dom.

Your code is way bloated, DOM 0 methods will be much faster.

If speed really matters, store spot + batchSize && i < cats.options.length in variables so they aren't re-calcuated on each loop (modern browsers probably don't, but older ones did):

for (var i=spot, j=spot+batchSize, k=cats.options.length; i < j && i < k; i++) {

    // Store reference to element
    var opt = cats.options[i];

    // The selected property is boolean, no need to pare
    if (opt.selected) {

        // if checkCatSICAdd() returns boolean, just use it
        // but maybe you need the boolean parison
        if (checkCatSICAdd(opt.name) === false) {

            // Wrapped for posting
            sel_cats.options[sel_cats.options.length] = 
                                         new Option(opt.value, opt.name);

        } else {
            failed++;
        }
    }
}

Currently I have a loop that updates the DOM in each iteration; I have learned this is a bad practice & you should update the DOM as little as possible for better speed.

So I was wondering how I go about editing the below so I can store all the elements in one element or something & then do a single DOM addition once the loop ends.

Here is the loop..

    for (var i = spot; i < spot + batchSize && i < cats.options.length; i++) {

        // Check if the cat is selected

        if (cats.options[i].selected == true) {

            // Set this category's values to some variables
            var cat_id = cats.options[i].getAttribute('value');
            var cat_name = cats.options[i].text;     

            if (checkCatSICAdd(cat_id) === false) {           

                // Now we create the new element
                var new_option = document.createElement('option');

                // Add attribute
                new_option.setAttribute('value',cat_id);

                // Create text node
                var new_text_node = document.createTextNode(cat_name);

                // Append new text node to new option element we created
                new_option.appendChild(new_text_node);

                // Append new option tag to select list
                sel_cats.appendChild(new_option);

            } else {
                failed++;
            }

        }

    }

Currently I have a loop that updates the DOM in each iteration; I have learned this is a bad practice & you should update the DOM as little as possible for better speed.

So I was wondering how I go about editing the below so I can store all the elements in one element or something & then do a single DOM addition once the loop ends.

Here is the loop..

    for (var i = spot; i < spot + batchSize && i < cats.options.length; i++) {

        // Check if the cat is selected

        if (cats.options[i].selected == true) {

            // Set this category's values to some variables
            var cat_id = cats.options[i].getAttribute('value');
            var cat_name = cats.options[i].text;     

            if (checkCatSICAdd(cat_id) === false) {           

                // Now we create the new element
                var new_option = document.createElement('option');

                // Add attribute
                new_option.setAttribute('value',cat_id);

                // Create text node
                var new_text_node = document.createTextNode(cat_name);

                // Append new text node to new option element we created
                new_option.appendChild(new_text_node);

                // Append new option tag to select list
                sel_cats.appendChild(new_option);

            } else {
                failed++;
            }

        }

    }
Share Improve this question edited Aug 28, 2012 at 19:18 GEOCHET 21.3k15 gold badges77 silver badges99 bronze badges asked Apr 4, 2011 at 21:30 BrettBrett 20.2k57 gold badges166 silver badges304 bronze badges
Add a ment  | 

5 Answers 5

Reset to default 3

Working with DOM element in the loop is slow - no matter if you attach them to the document or not. Attaching them at the end is a bit faster since only only redraw is required but it's still slow.

The proper way is generating a plain old string containing HTML and attaching this string to the DOM using the innerHTML property of a DOM element.

Your code should be ok. The DOM won't actually redraw until the Javascript has finished executing. However, if you've encountered a problem browser that does perform badly, you could try creating a new select before your loop that is not yet attached to the DOM, populating it as you are now, then replacing sel_cats with that new select at the end. That way, the DOM is only updated once.

Your way is good enough unless you have great many items added to sel_cats - you add to the DOM only once.

The only way to improve efficiency might be to store the options as raw HTML then assign that after the loop:

var arrHTML = [];
for (var i = spot; i < spot + batchSize && i < cats.options.length; i++) {
    // Check if the cat is selected
    if (cats.options[i].selected == true) {
        // Set this category's values to some variables
        var cat_id = cats.options[i].value;
        var cat_name = cats.options[i].text;     
        if (checkCatSICAdd(cat_id) === false) {           
            arrHTML.push("<option value=\"" + cat_id + "\">" + cat_name + "</option>";
        }
        else {
            failed++;
        }
    }
}
sel_cats.innerHTML = arrHTML.join("");

Once you have the select list assigned to a variable, remove it from the dom using removeChild on its parent tag. You can then use appendChild in the loop before adding the select list back into the dom.

Your code is way bloated, DOM 0 methods will be much faster.

If speed really matters, store spot + batchSize && i < cats.options.length in variables so they aren't re-calcuated on each loop (modern browsers probably don't, but older ones did):

for (var i=spot, j=spot+batchSize, k=cats.options.length; i < j && i < k; i++) {

    // Store reference to element
    var opt = cats.options[i];

    // The selected property is boolean, no need to pare
    if (opt.selected) {

        // if checkCatSICAdd() returns boolean, just use it
        // but maybe you need the boolean parison
        if (checkCatSICAdd(opt.name) === false) {

            // Wrapped for posting
            sel_cats.options[sel_cats.options.length] = 
                                         new Option(opt.value, opt.name);

        } else {
            failed++;
        }
    }
}

本文标签: javascriptStoring elements in memory to prevent updating the DOM too oftenStack Overflow