1
\$\begingroup\$

I've been optimizing my code and I always tend to cache common selectors like this (jQ):

function init(){

  var ob = $('#ob'),
      fg = ob.find('.fg');
      win = window,
      s1 = $('#s1'),
      s1em = s1.find('em'),
      bodyHtml = $('body,html'),
      fboxFirst = ob.find('.first'),
      _continue = $('#continue');

  /* ...lots of more functions and stuff
     here that uses the main variables */
}

init();

I do that since I don't want to limit the querying of the DOM to the stuff that actually is specific for an action:

ob.on('click', '.fbox', function(){

  // these need to be quried here
  var t = $(this),
      tp = t.parent(),
      allChild = tp.nextAll('.child'),
      tpNext = allChild.first(),
      nextClose = tpNext.find('h2');

  // no need to query the DOM for these
  bodyHtml.doThing(); // bodyHtml = common selector, cache outside scope
  fboxFirst.doThing2(); // also common, cache outside scope

})

For me this makes much sense but there is a memory hit since the GC (Garbage Collector) will have a harder time to release memory but there should be more performant due to the fact that less querying of the DOM has to be done on different actions that target the more general DOM elements.

The problem I'm having is that most guides recommend to localize everything but at the same time it doesn't make any sense to query the DOM each time for the same selectors where there is no need. What would the benefit be to localize everything except that any app/website/thing would use a little less memory?

Full code:

/*-----------------------------------------------------

    JS + VELOCITY VERBOSE / TMP

    + jquery 2.1.3
    + velocity.js
    + velocity.ui.js

-----------------------------------------------------*/


// INIT
;
(function($, window, document, undefined) {
    'use strict';




    /* SELECTOR CACHE ------------------*/

    var d = document,
        _ob = d.getElementById('ob'),
        ob = $(_ob),
        s1 = d.getElementById('s1'),
        body = d.querySelector('body,html'), // used for scrolling
        fboxFirst = _ob.querySelectorAll('.first'), // must re-select @ ajax
        _continue = d.getElementById('continue'),
        win = window,
        bestText = d.getElementById('bestText');




    /* HELPERS -------------------------------- */

    // endsWith
    if (typeof String.prototype.endsWith != 'function') {
        String.prototype.endsWith = function(str) {
            return this.slice(-str.length) == str;
        };
    }

    // regFX helper (velocity.js)
    function regFX(name, fx, ease) {
        var ease;
        if (name.endsWith('In')) {
            fx.opacity = [1, 0];
            ease = ob_aniEase;
        } else {
            fx.opacity = [0, 1];
            ease = ob_aniEaseOut;
        }
        fx.translateZ = 0;
        $.Velocity.RegisterUI(name, {
            defaultDuration: ob_animTime,
            calls: [
                [fx, 0, {
                    easing: ease
                }]
            ]
        })
    }




    /* ANIMATION DEFAULTS ------------------*/

    var ob_delayTime = 300,
        ob_animTime = 200,
        ob_doubleTime = 400,
        ob_longTime = 800,
        ob_aniEase = 'easeOutQuad',
        ob_aniEaseOut = 'easeInQuad';


    // create fx + set default
    regFX('fx.slideIn', {
        translateX: [0, -50]
    });
    regFX('fx.slideOut', {
        translateX: 50
    });

    var animIn = 'fx.slideIn',
        animOut = 'fx.slideOut';




    /*-----------------------------------------------------

        BINDS

    -----------------------------------------------------*/


    function ob_bindEvents() {
        ob.on('click', '.fbox', ob_open);
        ob.on('click', '.mtrigger', ob_openMulti);
        ob.on('click', '.child h2', ob_close);
        $(s1).on('click', ob_closeAll);

    }

    ob_bindEvents();




    /*-----------------------------------------------------

        FX FUNCTIONS
        require vars from 'animation defaults'

    -----------------------------------------------------*/


    // Show details + next flight
    function nextAnim(element, nextElement, arrow) {

            element.velocity('transition.slideDownIn', {
                duration: ob_animTime,
                delay: ob_animTime,
                display: 'block',
                complete: function() {
                    nextElement.velocity(animIn, {
                        delay: ob_delayTime,
                        complete: function() { // animate arrow @ h2
                            arrow.velocity('transition.slideDownIn', {
                                display: '',
                                duration: ob_doubleTime
                            });
                        }
                    });
                }
            });

        } // end nextAnim()




    // Scroll element with callback
    function scrollCallback(element, target, getOffset, callback) {

            var offPos = target.offset().top;
            offPos = offPos - getOffset;

            element.velocity('stop').velocity('scroll', {
                duration: ob_doubleTime,
                offset: offPos,
                complete: callback
            });

        } // end scrollCallback()




    // Check if last element
    function isLastBox(element) {
            if (element.length < 1) {
                $(_continue).velocity(animIn, {
                    delay: ob_doubleTime * 2
                });
            }
        } // end()




    // Button push
    function buttonPush(e, callback) {
            e.velocity('stop').velocity({
                scale: [0.90, 1]
            }, {
                duration: 60
            }).velocity({
                scale: 1
            }, {
                duration: 60,
                complete: function() {
                    if (callback) {
                        callback();
                    }
                }
            });
        } // end()



    // scrollTop action (re-use scrollCallback instead)
    function ob_scrollTop(e) {
            var curTop = e.offset().top - 100;
            $(body).velocity('stop').velocity('scroll', {
                duration: ob_doubleTime,
                offset: curTop
            }, {
                easing: ob_aniEase
            })
        } // end()




    /*-----------------------------------------------------

        OPEN FUNCTIONS

    -----------------------------------------------------*/



    // SINGLE OPEN ---------------------------------------

    function ob_open() {

            // VARs : this-specific
            var t = $(this),
                tp = t.parent(),
                allChild = tp.nextAll('.child'), // all .child after fbox 
                tpNext = allChild.first(), // first .child after current .fbox
                nextClose = tpNext.find('h2'), // ### fix h2 em prev.Value mux later ###
                prevClose = tp.find('h2'),
                nextDetails = t.next('.details'); // next .details

            // CHECKS
            var _isChild = false;
            if (t.hasClass('mchild')) {
                _isChild = true;
            }
            var _isMulti = false;
            if (tp.hasClass('multi')) {
                _isMulti = true;
            }


            // Single outbound flights
            if (!_isMulti || _isChild) {

                // CHECKS
                var _isFirst;
                if (tp.hasClass('first')) {
                    _isFirst = true;
                } else {
                    _isFirst = false;
                }


                // ACTIONS
                this.className += ' on';
                bestText.style.display = 'none'; // fix

                // _isFirst
                if (_isFirst) {

                    $(fboxFirst).addClass('off'); // hide all .first (.first's are outside of current flightGroup)
                    tp.removeClass('off'); // ..but not the current
                    $(s1).addClass('on'); // show X-mark at first

                    if (_isChild) { // hide all .fbox, but first
                        tp.find('.fbox').hide();
                        t.show();
                    }

                } else {

                    tp.find('h2').addClass('on');
                    tp.find('.fbox').addClass('off'); // set OFF on all .fbox in current group (needed for all .child)
                    t.removeClass('off'); // remove on current

                } // end _isFirst


                var fa = tpNext.find('h2 .fa');
                fa.css('opacity', 0); //tmp


                // Scroll to 'this' + run nextAnim()
                scrollCallback($(body), t, 60, nextAnim(nextDetails, tpNext, fa));

                // Check if last .fbox + run action
                isLastBox(allChild);

            } // end Single outbound flights

        } // end ob_open()




    // MULTI OPEN ---------------------------------------

    function ob_openMulti() {

            // VARs : this-specific
            var t = $(this),
                tp = t.parent(),
                mchild = tp.find('.mchild'),
                arrow = t.find('.arrow');



            // checks
            var isOn = false;
            if (t.hasClass('on')) {
                isOn = true;
            }

            if (isOn) {
                buttonPush(arrow, function() {
                    mchild.velocity(animOut);
                    arrow.velocity({
                        rotateZ: '0deg'
                    }, ob_animTime);
                });
            } else {
                ob_scrollTop(t);
                buttonPush(arrow, function() {
                    mchild.velocity(animIn, {
                        delay: ob_animTime * 1.2
                    });
                    arrow.velocity({
                        rotateZ: '180deg'
                    }, ob_animTime);
                })
            }

            t.toggleClass('on');


        } // end ob_openMulti()




    /* -----------------------------------------------------

        CLOSE FUNCTIONS

    ----------------------------------------------------- */



    // CLOSE DEFAULT  --------------------------------------

    function ob_close(e) {

            var t = $(this),
                tp = t.parent(),
                tpNextAll = tp.nextAll('.child');

            // actions
            //t.addClass('off');
            t[0].className += ' off'; // hide X-mark on current

            tp.velocity(animOut, {
                complete: function() {

                    // NextAll resets
                    tp.find('.fbox').removeClass('off on'); // remove local .off/.on
                    tp.find('.details').hide(); // hide .details on current
                    tpNextAll.find('.details').hide(); // hide .details on nextAll .child
                    tpNextAll.children('.fbox').removeClass('off on'); // reset hide in ALL .child(s)


                    // Change header text
                    var h2next = tpNextAll.find('h2');
                    t.removeClass('on off'); // fix?
                    h2next.removeClass('on'); // fix?

                    // Animate back
                    tp.velocity(animIn); // show current flightGroup again

                }
            });

            $(_continue).velocity(animOut);

        } // end ob_close()




    // CLOSE ALL ----------------------------------------


    function ob_closeAll() {

            var t = $(this),
                fg = ob.find('.fg'),
                all = fg.add($(s1));

            // checks
            var mChilds = $(fboxFirst).find('.mchild');
            var _isChild = false;
            if (mChilds.hasClass('on')) {
                _isChild = true;
            }

            // add bestText
            bestText.style.opacity = 0;
            all = all.add($(bestText));

            // fix carrier flicker
            var carrier = $(fboxFirst).find('.carrier'); // limit to .fbox.first only

            // Animations
            all.stop().velocity(animOut, {
                display: false,
                stagger: ob_animTime / 4,

                // everything animated out
                complete: function() {

                        var h2 = ob.find('h2'),
                            fbox = ob.find('.fbox'),
                            details = ob.find('.details'),
                            child = ob.find('.child');

                        // perform functions
                        details.add(child).hide();
                        h2.add(fbox).add($(fboxFirst)).removeClass('on off');

                        // show best text
                        bestText.style.display = '';

                        // hide carrier, else flickers
                        carrier.hide();

                        // MULTI-CHILD CLOSE
                        if (_isChild) {
                            var mTrigger = $(fboxFirst).find('.mtrigger');
                            mTrigger.removeClass('on').show();
                            mTrigger.find('.arrow')[0].removeAttribute('style'); // clean styles
                            mChilds.hide();
                            all.add(mTrigger);
                        }

                        // perform IN-animation
                        all.velocity(animIn, {
                            display: false,
                            stagger: ob_animTime / 4,
                            complete: function() {
                                carrier.show();
                            }
                        });

                    } //onComplete

            })

            $(_continue).velocity(animOut);


        } // end closeAll()




})(jQuery, window, document); // end() init
\$\endgroup\$
  • \$\begingroup\$ Welcome to CR! It's best if you include your real code - including the real comments; everything in a code block is subject to be peer reviewed on this site. Are these comments in your actual code? If they're in just for context, it's best to leave them out and explain the context in plain text - as it stands this question is bordering the line of hypothetical/example code; good CR questions put the code front and center, not just as a pretext for a discussion on best practices or brainstorming ideas. \$\endgroup\$ – Mathieu Guindon Mar 3 '15 at 17:08
  • \$\begingroup\$ The current codebase isn't that clean since it also contains a lot of temporary demo-stuff etc and is about a mile longer. I thought a slimmer version that highlights the relevant stuff was much better. \$\endgroup\$ – Tommie Mar 3 '15 at 17:24
  • \$\begingroup\$ On a site like Stack Overflow, indeed it's better to keep it short and to-the-point. Here on Code Review, everything in your code is relevant, reviewers are always free to comment on any aspect of the code - hence it's best to just post your code as it is, and let the accompanying text tell us what your concerns are, to guide reviewers toward the type of answer you're willing to mark as accepted :) \$\endgroup\$ – Mathieu Guindon Mar 3 '15 at 17:29
1
\$\begingroup\$

There is a benefit to using local variables besides memory savings, namely avoiding polluting the global namespace.

You can use closures to adhere to the best practice of using only local variables without having to re-query the DOM for elements you've already selected.

Using closures to maintain references to objects

Although it'll still consume as much memory as if you kept the variables "cached" in global variables, you can modify your code using closures to maintain references to local variable values even after they've gone out of scope, as in the example below.

function init(){
    var ob = $('#ob'), bodyHtml = $('body,html'), fboxFirst = ob.find('.first');

    /* ...more functions and stuff
    here that use the main variables */

    (function(local_bodyHtml, local_fboxFirst){ 
        // Note: I renamed the local variables here just to demonstrate that the
        // local handles can be different from the objects passed as parameters.
        // You can keep the original names instead.

        ob.on('click', '.fbox', function(){

            // these need to be queried here
            var t = $(this),
            tp = t.parent(),
            allChild = tp.nextAll('.child'),
            tpNext = allChild.first(),
            nextClose = tpNext.find('h2');

            // no need to query the DOM for these
            local_bodyHtml.doThing(); 
            local_fboxFirst.doThing2(); 
        })
    })(bodyHtml,fboxFirst); 
    // References to elements bodyHtml and fboxFirst are captured in the closure 
    // by adding them as parameters to this immediately-invoking function expression.
}

Here's a working snippet that demonstrates another method of using closures to maintain live references to HTML elements.

function init() {
    var ob = document.getElementById('ob');
    var fboxFirst = ob.querySelector('.first');
    var fboxes = ob.querySelectorAll(".fbox");

    for (var i = 0; i < fboxes.length; i++) {
        fboxes[i].onclick = fboxClickHandler(fboxFirst);
        /* Note: overriding onclick will clobber existing event handlers, only using it here for readability */
    }
}

function fboxClickHandler(fboxFirst) {
    return function () {
        if (this === fboxFirst) {
            alert("I'm " + fboxFirst.innerHTML + "!");
        } else {
            alert("I wish I were " + fboxFirst.innerHTML + " but instead I'm " + this.innerHTML + ".");
        }
    };
}
init();
.fbox{display:inline-block;background-color:#dfdfdf;text-align:center;border:1px solid black; margin:1px;width:100px;}
.fbox:hover{cursor:pointer;background-color:white;}
Change the first box's inner html: <input type="text" id="input" onkeyup="document.querySelector('.first').innerHTML = document.getElementById('input').value;" value="first"/>
<div id="ob">
    <div class="first fbox">first</div>
    <div class="fbox">second</div>
    <div class="fbox">third</div>
    <div class="fbox">fourth</div>
    <div class="fbox">fifth</div>
</div>
Notice that each box's click event handler is able to reference the inner HTML of the first box, even though the handler does not query the DOM to grab the element and no global reference to the object is maintained in the script.

Improving performance by using native methods whenever possible

Another thing you'll want to consider if you're worried about code performance is whether you can replace any of the jQuery shortcut functions with the analogous native methods. For example, document.getElementById("elementid") will always outperform $("#elementid"), and assuming they're supported by the browser document.querySelector(".cssSelector") and document.querySelectorAll(".cssSelector") will always outperform $(".cssSelector").

querySelector and querySelectorAll can be called on any element (not just document) so you can also replace jQuery's element.find(".cssSelector") method for similar performance gains.

The native methods will also increase the verbosity of your code (and thus the number of bytes that must be downloaded), so take that advice with a grain of salt; personally, I find that the dedicated native methods also make the code more readable and thus maintainable.

\$\endgroup\$
  • 1
    \$\begingroup\$ Something worth mentioning as an addendum to "using native methods whenever possible", is that it's never a bad thing to head over to jsperf.com, and put in some test cases for different ways of doing things. Ridiculously, most of the functions that iterate arrays in current Javascript engines (foreach, map, filter) are slower than doing a simple JS for loop. So while that's always a good rule of thumb when dealing with Javascript or any other language like Javascript, it's not 100% accurate (more like 99.5 or so) \$\endgroup\$ – Eric Blade Mar 4 '15 at 18:25
  • \$\begingroup\$ That's a great point: jsperf.com is invaluable for evaluating different techniques. It's how I learned to use element.insertAdjacentHtml("beforeend",myHtmlString); instead of element.innerHTML += myHtmlString;. It also demonstrated that Firefox 35.0 has a serious problem with its implementation of Array.splice() \$\endgroup\$ – Thriggle Mar 4 '15 at 19:21
  • \$\begingroup\$ ... insertAdjacentHtml ? how do people even find these things? :-D \$\endgroup\$ – Eric Blade Mar 5 '15 at 0:56
  • \$\begingroup\$ I don't really understand your answer since i already use closures? I'm also well aware of the JQ-vs-native but don't want to initially litter the code with for(var i=0;selector.length<i;i++){ selector[i].doStuff(); } My Q was more about caching the selectors vs not in the way i currently do it. \$\endgroup\$ – Tommie Mar 5 '15 at 13:50

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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