Resizing a sidebar

Question

This is really simple but I would like to improve the efficiency as it's run frequently on the client:

function resizeSidebar() {

    var h_tmp = 0;

    if ($("#internal-holder").length > 0)
    { h_tmp = 250; }
    else
    { h_tmp = 181; }

    var h_var = $(window).height() - h_tmp;
    var h_fixed = $(window).height();

    $("#aside-holder").css("height", h_fixed - 225 + "px");
    $("#main-content").css("height", h_var + "px");
    $("#main-inner").css("height", h_var - 115 + "px");
    $("#main-inner-scroll .viewport").css("height", h_var - 110 + "px");
    $("#aside-holder-scroll .viewport").css("height", h_fixed - 225 + "px");
    $("#main-inner-scroll").tinyscrollbar();
    if (hideAsideScrollBar != "true") {
        $("#aside-holder-scroll").tinyscrollbar();
    }
}

Answer 1

1. $("#foo") is slow, use the DOM: document.getElementById("foo").

2. $("#foo .bar") is slow, get rid of that by navigating the DOM. I can't say exactly how without seeing the markup, but look at the FIXME comments below.

3. $("#foo").css() is slow, use the DOM: foo.style.height = "24px".

4. Get the reference to the elements once, not every time the function is called.

Other notes:

1. Don't do that weird thing with the braces on your if statements.

2. Use the var keyword as little as possible by combining variable declarations.

   1. This appeases linters and reduces minified file size.

   2. It also pretty much forces you to declare all your variables at the top of the function, which is generally considered to be a good practice.

3. Avoid the use of _ in variable names; prefer camelCase instead. Also try to use meaningful variable names.

   1. Except for variables or properties representing constants. In these cases the UPPER_CASED words are joined by underscores.

   2. An exception may be made for variables or properties meant to be considered "private". In these cases the camelCased words are prefixed or suffixed with an underscore.

---

var internalHolder = document.getElementById("internal-holder"),
    asideHolder = document.getElementById("aside-holder"),
    mainContent = document.getElementById("main-content"),
    mainInner = document.getElementById("main-inner"),
    mainInnerScroll = document.getElementById("main-inner-scroll"),
    asideHolderScroll = document.getElementById("aside-holder-scroll"),
    mainInnerScrollViewport = mainInnerScroll.children[0], // FIXME  
    asideHolderScrollViewport = asideHolderScroll.children[0]; // FIXME  

function resizeSidebar() {

    var tmpHeight = internalHolder.children.length ? 250 : 181,
        height = window.innerHeight,
        innerHeight = height - tmpHeight;

    asideHolder.style.height = height - 225 + "px";
    mainContent.style.height = innerHeight + "px";
    mainInner.style.height = innerHeight - 115 + "px";

    mainInnerScrollViewport.style.height = innerHeight - 110 + "px";
    asideHolderScrollViewport.style.height = height - 225 + "px";

    $(mainInnerScroll).tinyscrollbar();
    if (hideAsideScrollBar != "true") {
        $(asideHolderScroll).tinyscrollbar();
    }
}

---

I can almost guarantee this will be at least 10 times faster if you fix the FIXMEs.

BUT.

What you are doing feels really wrong. Using javascript to get and set a bunch of heights should be avoidable. Again I can't say exactly what to do without seeing the markup, but you should be able to achieve most (if not all) of the $('#foo').css stuff through plain old HTML/CSS. Try giving things 100% height, try inline-block and vertical-align, hell, try tables, but javascript css styling should, IMO, be kept to an absolute minimum.

Answer 2

If the items which you are selecting in the function are a part of the same container, you must avoid to do full lookups in DOM. Look at your html, select the parent container in which you do the manipulations, and store it's value. After that, just search for it's children for desired components.

For example, you have the following snippet:

...
<div class="container">
  <div class="someItem">
    <div class="someOtherItem">
...

Now, if you have a function that is called pretty often, the ideal case is to select the elements in the following manner:

function doSomething () {
  var element = $('.container')
  var someItem = element.find('.someItem')
  var otherItem = someItem.find('.someOtherItem')

  someItem.doSomeStuff()
  otherItem.doSomeStuff()
}