5
\$\begingroup\$

I have a small piece of code, and the problem is that I don't know how to organize it. There are tons of tutorials about JS code organization but I feel that those are for large scale apps. Also, I'm not sure if having the setInterval function where it is now is a big no no, can you come up with some critique/ideas?

YUI().use('node', function (Y) {
    // Access DOM nodes.

    var chapterTime = [0, 30, 49, 68, 90, 105, 127, 153, 182, 209],
        buttons = Y.one('ul'),
        currentChapter = 0,
        prevChapter = 0,
        myPlayer = videojs('example_video_1');

    buttons.delegate('click', function () {
        var button_name =  this.ancestor().get('id'),
            chapter = button_name.slice(1);

            playChapter(chapterTime[chapter-1], this);
    }, 'a');    

    function playChapter(chTime, chButton) {        
        myPlayer.play();
        setTimeout(function(){ myPlayer.currentTime(chTime); },100);
        Y.all('#nav a').setStyle('backgroundColor', "#999");
        chButton.setStyle('backgroundColor', '#666');
    }


    setInterval(function(){
        var playbackTime = myPlayer.currentTime(),
            i = 0,
            chapterFound = false;
            //console.log(chapterTime);


        while (i <= 9 && chapterFound == false){
            if (playbackTime >= chapterTime[i] && playbackTime < chapterTime[i+1]){
                currentChapter = i + 1;
                chapterFound = true;
            } else {
                i++;
            }
        } 

        if (currentChapter != prevChapter){
            var button = Y.one('#c' + currentChapter);
            Y.all('#nav a').setStyle('backgroundColor', '#999');
            button.one('a').setStyle('backgroundColor', '#666');
            prevChapter = currentChapter;
        }
    },1000);
});
\$\endgroup\$
1
\$\begingroup\$

This looks just fine to me.

Some very minor nitpicks:

  • Remove //console.log(chapterTime);
  • Consider removing the blank line after var statements, it does not add much
  • chapterTime could use a comment as to what those numbers express (I assume seconds)
  • I would probably write this

    buttons.delegate('click', function () {
        var button_name =  this.ancestor().get('id'),
            chapter = button_name.slice(1);
    
            playChapter(chapterTime[chapter-1], this);
    }, 'a');   
    

    as

    buttons.delegate('click', function () {
            var chapter = this.ancestor().get('id').slice(1) - 1;
            playChapter(chapterTime[chapter], this);
    }, 'a');   
    
  • The color strings #999 and #666 have a meaning, I assume enabled/disabled, you could use properly named constants for these, declared on top
  • Only seldom use double newlines, they are vertical overkill

Still, all in all I like your code.

\$\endgroup\$

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.