Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm trying to improve the quality/efficiency of my coding (having only been learning jQuery for a couple of months).

I have a function which loops through all $('.loop-bar')'s and applies a jQueryUI slider to each. Each loop bar is a range slider which controls the portion of each song (playing via a custom player i have developed using soundmanager 2).

There's a lot of other stuff it is doing at the same time such as adjusting the progress bar width, resetting it, adding data to the DOM in order to perform loop checks etc.

However it strikes me that I am having to declare an awful lot of variables. Are there any shortcuts/ways I can improve this?

For those of you with a high level of jquery experience, have i fallen into any common pitfalls, does the performance look optimal (it performs absolutely fine in real world but could it be improved?).

Thanks all.

function initLoopSliders(oTable) {   

        oTable.find('.loop-bar').each(function () {

            var $this = $(this);
            var $seekbar = $this.parent().siblings('.player-timeline').find('.seek-bar');
            var $seekhead = $('#seekhead');
            var $widthgauge = $seekbar.siblings('ol');
            var sliderwidth = $widthgauge.width();
            var $datastore = $this.closest('td');
            var soundID = $datastore.find('.playable').data('playable').sID;
            var $progressbar = $datastore.find('.position');
            var $body = $('body');
            var $righthandle;
            var $lefthandle;
            var $songdata = $datastore.data('data');
            var $fullduration = $datastore.attr('alt');

            $this.parent().width(sliderwidth);

            $this.slider({
                range: true,
                min: 0,
                max: sliderwidth,
                values: [0, sliderwidth],
                start: function (event, ui) {
                    $body.attr('looping', soundID).attr('searching', 'true');
                    $lefthandle = $this.find('.loop-start');
                    $righthandle = $this.find('.loop-end');

                    soundManager.stop(soundID);
                    $progressbar.width(0);
                },
                slide: function (event, ui) {

                    loopstart = rectime($songdata.loopstart);
                    loopend = rectime($songdata.loopend);
                    left = ui.values[0];
                    seekheadLeft = $seekbar.offset().left;
                    rightwidth = sliderwidth - ui.values[1];
                    width = sliderwidth - left;
                    newwidth = width - rightwidth;

                    $seekbar.css({
                        left: left + 'px',
                        width: newwidth + 'px'
                    });
                    $seekhead.css({
                        left: seekheadLeft + 'px',
                        width: newwidth + 'px'
                    });

                    $songdata.loopduration = $fullduration * (newwidth / sliderwidth);
                    $songdata.relativeduration = newwidth / sliderwidth;
                    $songdata.loopstart = Math.round(($fullduration / sliderwidth) * ui.values[0]);
                    $songdata.loopend = Math.round(($fullduration / sliderwidth) * ui.values[1]);

                    $lefthandle.text(loopstart);
                    $righthandle.text(loopend);
                    $datastore.data('data', $songdata);
                },
                stop: function (event, ui) {

                    $body.attr('searching', 'false');

                    if (ui.values[0] == 0 && ui.values[1] == sliderwidth) {
                        $object = $(this).find('a');
                        soundManager.play(soundID);
                        hideLoopHandle2($object);
                        $body.attr('looping', '');
                    }
                    else {
                        from = Number(($datastore.data('data').loopstart) * 1000);
                        to = Number(($datastore.data('data').loopend) * 1000);
                        playFromTo(from, to, soundID);
                    }
                }
            });

        }); //end each


    // Add hoverintent to handles
        $('.loop-bar').find('a').hoverIntent({
            sensitivity: 10,
            // How sensitive the hoverIntent should be
            interval: 10,
            // How often in milliseconds the onmouseover should be checked
            over: showLoopHandle,
            // Function to call when mouseover is called    
            timeout: 500,
            // How often in milliseconds the onmouseout should be checked
            out: hideLoopHandle // Function to call when mouseout is called    
        });


    } // end
share|improve this question

1 Answer

up vote 2 down vote accepted

it performs absolutely fine in real world

And

have i fallen into any common pitfalls

Yes, you have. It's called premature optimization. The code looks fine, it's not hard to understand. You are just missing a couple comments to explain what the function is for.

share|improve this answer
Thanks Sylverdrag... Thats all I needed to hear. – user6595 Jan 22 '12 at 22:03

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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