-3
\$\begingroup\$

My application hangs up on IE and mobile browsers when these functions are fired. Is there anything that stands out as being obviously performance-killing?

$this.find('input.bundle-check').live('change', function() {
    var $box = $(this),
    ntn = $box.data().ntn,
    price = $box.data().price,
    savings = $box.data().savings;
    if ($box.is(':checked')) {
        productsBundled[ntn] = {
            "price"     :   price,
            "savings"   :   savings,
            "ntnid"     :   ntn,
            "qty"       :   1
        }; 
        $box.siblings('label').text(' Selected')
        $box.closest('.grid-product').fadeTo(300, 0.5)
    } else {
        $box.siblings('label').text(' Add Item');
        delete productsBundled[ntn];
        $box.closest('.grid-product').fadeTo(300, 1.0)
    }
    refreshSelectedItems(productsBundled);
    $this.find('.itemCount').text(concat('(',objectCount(productsBundled),')'));

})

function refreshSelectedItems(products, remote) {
    var itemntns = [], totalPrice=0.00, totalSavings=0.00;
    products = products || {};
    remote = remote || 2;
    if (objectCount(products) > 0) {
        $.each(products, function(i, item) {
            $qtyBox = $('.selected-item[data-ntn=' + i + '] .cartqty');
            itemntns.push(i);
            totalPrice += (item.price * ($qtyBox.val() || 1));
            totalSavings += (item.savings * ($qtyBox.val() || 1));
                    // console.log('qtyBox', $qtyBox.val())
                }); 

        if(remote > 1) {
            $.ajax({
                url: '/Includes/pageHelper.cfc',
                type: 'post',
                async: true,
                data: {
                    method: "getBundleSelectedItems",
                    productList: itemntns.join(',')
                },
                success: function(data) {
                    var $container = $('.selected-items > span');
                    $container.html(data);
                    $.each($container.find('.selected-item'), function() {
                        var myntn = $(this).data().ntn,
                        $price = $(this).find('.price'),
                        $bunPrice = $('<span />').addClass('bundle-price');

                        $(this).find('.cartqty').val(productsBundled[myntn].qty);

                        if (productsBundled[myntn].savings > 0) {
                            $bunPrice.text(concat(' $', productsBundled[myntn].price.toFixed(2)));
                            $(this).find('em').hide();
                            $price.after($bunPrice.after($('<span />').addClass('sale').text(concat(' You save $', productsBundled[myntn].savings.toFixed(2), '!'))));
                        }
                    })
                }
            })
        }
    } else {
        $('.selected-items > span').html('');
    }

    $this.find('.bundle-saving').text(concat("$", totalSavings.toFixed(2)));
    $this.find('.bundle-addons').text(concat("$", totalPrice.toFixed(2)));
    totalPrice = totalPrice + (parseFloat($('.original-products > div:has(:checked)').data().price) * parseInt($('.original-products > div:has(:checked) .cartqty').val()));
    $this.find('.bundle-total').text(concat("$", totalPrice.toFixed(2)));

}
\$\endgroup\$

closed as off-topic by t3chb0t, Mast, hjpotter92, Toby Speight, Stephen Rauch Apr 2 '18 at 12:58

This question appears to be off-topic. The users who voted to close gave these specific reasons:

  • "Code not implemented or not working as intended: Code Review is a community where programmers peer-review your working code to address issues such as security, maintainability, performance, and scalability. We require that the code be working correctly, to the best of the author's knowledge, before proceeding with a review." – Mast, Stephen Rauch
  • "Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." – t3chb0t, hjpotter92
If this question can be reworded to fit the rules in the help center, please edit the question.

  • 1
    \$\begingroup\$ well... live() should no longer be used.. it is deprecated. In newer versions use .on() and in older versions use .delegate() - not sure if this will impact performance that much, but .live() is known to be slow. \$\endgroup\$ – rlemon Aug 15 '12 at 14:44
  • \$\begingroup\$ I typically use on, but this content can be loaded with AJAX. maybe this was too much of a shortcut to simply rebinding? \$\endgroup\$ – Kyle Macey Aug 15 '12 at 14:47
  • \$\begingroup\$ @KyleMacey If the content can be added after the handler is attached, then bind to a parent element [that is always there] and provide the selector of the children to bind the handler to: $this.on('click', 'input.bundle-check', handler); . The API explains this. \$\endgroup\$ – ANeves Aug 15 '12 at 15:31
  • \$\begingroup\$ I always wondered what the advantage was of that syntax... thanks \$\endgroup\$ – Kyle Macey Aug 15 '12 at 15:32
  • \$\begingroup\$ it would probably be quicker to use a for() loop instead of $.each(products, function(i, item) { \$\endgroup\$ – Zathrus Writer Aug 16 '12 at 6:50
1
\$\begingroup\$

There are a few things which stand out to me:

var $box = $(this),
ntn = $box.data().ntn,
price = $box.data().price,
savings = $box.data().savings;

You use $box.data() several times; it may be faster to cache the return value of that function.

if ($box.is(':checked')) {

:checked is not a standard CSS selector, so it will be slower than simply:

if (this.checked) {

Later, you used

                $.each($container.find('.selected-item'), function() {

I'm not sure if it's faster, but you could just do:

                $container.find('.selected-item').each(function() {

Finally, this line:

totalPrice = totalPrice + (parseFloat($('.original-products > div:has(:checked)').data().price) * parseInt($('.original-products > div:has(:checked) .cartqty').val()));

The :has selector is not standard CSS, so it can't use browsers' native functions. instead, consider using the has() method instead:

totalPrice = totalPrice + (parseFloat($('.original-products').children('div').has(':checked').data().price) * parseInt($('.original-products').children('div').has(':checked').find('.cartqty').val(), 10));

Note that I also added the radix argument to parseInt. A micro-optimization at best, but it does mean the JS engine doesn't need to guess.

Hope that helps.

\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.