0
\$\begingroup\$

I'm trying to get in the habit of writing better looking code and more efficient code blocks. I wrote this Twitch TV Application which allows you to add and edit channels and it lets you know when a channel is live. Is there anything I can work on to make my code work / look better?

HTML

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
    <title>Streaming</title>
    <link href="style.css" rel="stylesheet" type="text/css" />
    <script type="text/javascript" src="//ajax.googleapis.com/ajax/libs/jquery/1.9.1/jquery.min.js"></script>
    <script type="text/javascript" src="streamer.js"></script>
    <script type="text/javascript" src="dynamic.js"></script>
</head>
<body>
    <div id="container">
        <div id="nav">
            <ul id="mainNav">
                <li id="streamInfo"></li>
            </ul>
            <ul id="chatAnchor">
                <li>
                    <a onclick="$(this).addClass('activeChat');$('#streamChat').removeClass('activeChat')" href="http://twitch.tv/chat/embed?popout_chat=true&amp;channel=mxgdichello" target="chatFrame" id="mainChat" class="activeChat">
                        Main Chat
                    </a>
                </li>
                <li>
                    <a href="javascript: void(0)" class="tab" id="streamChat" onclick="changeChat()">
                        Stream Chat
                    </a>
                </li>
            </ul>
        </div>
        <div style="clear:both"></div>
        <div id="player">
            <object type="application/x-shockwave-flash" id="twitchTV" width="780px" 
            height="500px"></object>
        </div>
        <div id="chat">
            <iframe name="chatFrame" frameborder="0" scrolling="no" id="chatFrame" src="http://twitch.tv/chat/embed?popout_chat=true&amp;channel=mxgdichello"></iframe>
        </div> 
        <div id="chanControls">
            <div id="add">
                <a href="javascript:void(0)">
                    <img src="add.jpg" />
                </a>
                <form id="addChan" onSubmit="return addChannel()">
                    Streamer Name: 
                        <input type="text" name="title" />
                    Streamer Channel:
                        <span style="color:#383838; font-style:italic">
                            http://www.twitch.tv/
                        </span>
                        <input type="text" name="url" />
                    <input type="submit" id="submit" value="Verify Channel" />
                    <span class="successMsg"></span>
                </form>
            </div>
            <div id="edit">
                <a href="javascript:void(0)">
                    <img src="edit.jpg" />
                </a>
            </div>
        </div>
        <div id="list">
            <ul></ul>
        </div>
    </div>

</body>
</html>

Jquery

var streams = new Array();
var current = -1; // For viewers update
var twitch = false;
var youtube = false;
var timer = 60000; // Miliseconds
var toggleEdit = [editList, addList];
var toggleCounter = 0;         
var youtubePlayer = '<iframe width="765" height="500" frameborder="0" allowfullscreen id="youtubePlayer"></iframe>';
var twitchPlayer = '<object type="application/x-shockwave-flash" id="twitchTV" width="780px" height="500px"></object>';
var tubes = ["zn7-fVtT16k", "IVJVCoHDAXs", "1_hKLfTKU5Y", "l3w2MTXBebg", "UcTLJ692F70", "zj2Zf9tlg2Y", "mgVwv0ZuPhM",
            "7ZsKqbt3gQ0", "YHRxv-40WMU", "ZIMoQHpvFQQ", "TAaE7sJahiw", "xBzoBgfm55w", "AeNYDwbm9qw", "mhTd4_Ids80",
            "AFA-rOls8YA", "CeLrlmV9A-s", "7rE0-ek6MZA", "WA4tLCGcTG4", "0M0RbaPxq2k", "vICX-6dMOuA", "njos57IJf-0",
            "K5a_v0MP_Fk", "dX_1B0w7Hzc", "xDj7gvc_dsA", "17CLlZuiBkQ", "0kRAKXFrYQ4", "gJ1Mz7kGVf0", "-6G6CZT7h4",
            "liLU2tEz7KY", "eHCyaJS4Cbs", "uEIPCOwY4DE", "V2XGp5ix8HE", "DrQRS40OKNE", "BBcYG_J3O2Q", "upxzaVMhw8k",
            "0XcN12uVHeQ", "itvJybdcYbI", "l7iVsdRbhnc", "51V1VMkuyx0", "aQQeg3jYgOA", "XGK84Poeynk", "MaCZN2N6Q_I",
            "9q5pZ49r9aU", "DFM140rju4k", "qHBVnMf2t7w", "YtO-6Xg3g2M"];
var updateTimer = setInterval(getList, timer);          // Update the streams list every minute

$(window).load(function(){
    if(localStorage.length == 0)
    {
        var streamer1 = new Streamer("Gaurdsman Bob", "guardsmanbob", null, null);
        var streamer2 = new Streamer("Siv HD", "sivhd", null, null);
        var streamer3 = new Streamer("Day9 TV", "day9tv", null, null);
        localStorage.setItem(streamer1.url, JSON.stringify(streamer1));
        localStorage.setItem(streamer2.url, JSON.stringify(streamer2));
        localStorage.setItem(streamer3.url, JSON.stringify(streamer3));
    }

    getStorage();
    getList();
});

$(document).ready(function(){
    $('#add a').click(function() {
        var lefty = $(this).next();
        lefty.animate({
          left: parseInt(lefty.css('left'),10) == 150 ?
            -lefty.width() : 150 
        });
    });

    $('#addChan input[name=title]').click(function(){
        var title = $('#addChan input[name=title]');
        if(title.val() == "Enter Streamer Name")
        {
            title.val('');
            title.css("border-color", "");
        }
    });

    $('#addChan input[name=url]').click(function(){
        var url = $('#addChan input[name=url]');
        if(url.val() == "Enter Streamer URL")
        {
            url.val('');
            url.css("border-color", "");
        }
    });

    $('#edit a').click(function(){
        if(toggleCounter % 2 == 1)
        {
            $(document).off('click', '#list li img');
            $(document).off('focusout', '#list input.editTitle');
        }

        toggleEdit[toggleCounter++%2]();
    });
});

function getList(){
    $.post(
        "streams.php",
        {streams : JSON.stringify(streams)}, 
        function(data) 
        {
            streams = $.parseJSON(data);

            $.each(streams, function(index, obj){ 
                $.each(obj, function(key, value){
                    if(current == -1 && value == 'online')
                    {
                        current = index;
                        return false;
                    }
                });
            });

            if(current != -1 && !twitch)
                build(current);
            else if(current == -1 && !youtube)
                randomTube();

            if(twitch)
                updateViewers();
            addList();
        }
      );
}

function build(index){
    var data = 'http://www.twitch.tv/widgets/live_embed_player.swf?channel=';   // Object Data
    var src = 'hostname=www.twitch.tv&auto_play=false&start_volume=25&channel='; // Flashvars Param
    var changeVars = '<param name="flashvars" \
    value="hostname=www.twitch.tv&auto_play=false&start_volume=100&channel='+streams[index].url+'"/>';
    var params = '<param name="allowFullScreen" value="true" />' +
                 '<param name="allowScriptAccess" value="always" />' +
                 '<param name="allowNetworking" value="all" />' +
                 '<param name="movie" value="http://www.twitch.tv/widgets/live_embed_player.swf" />' +
                 changeVars;
    $("#player").html(twitchPlayer);
    $("#twitchTV").html(params);
    $("#twitchTV").attr("data", data);
    if(streams[index].status == 'online')
        $('#streamInfo').html("<span id=\"streamTitle\">Streamer: " + streams[index].title + 
            "</span> - <span id=\"viewers\">" + streams[index].viewers + "</span> Viewers");
    else
        $('#streamInfo').text("Streamer: " + streams[index].title + " - Offline");

    current = index;
    twitch = true;
}

function addList(){
    var numOffline = 0;
    var online = '';
    var offline = '';
    var curr = '';

    for(var i = 0; i < streams.length; i++)
    {
        if(i == current && streams[i].status == 'online') 
        {
                curr = '<li class="item"><div class="online"></div><a style="color:green" href="javascript: void(0)" \
                        title="'+streams[i].title+' Stream" \
                        onClick="changeStream($(this).text())">'+streams[i].title+'</a></li>';
        }
        else if(streams[i].status == 'online' && i != current)
        {
                online += '<li class="item"><div class="online"></div><a style="color:green" href="javascript: void(0)" \
                        title="'+streams[i].title+' Stream" \
                        onClick="changeStream($(this).text())">'+streams[i].title+'</a></li>';
        }
        else
        {
            offline += '<li class="item"><div class="offline"></div><a href="javascript: void(0)" \
                       title="'+streams[i].title+' Stream - Offline" \
                       onClick="changeStream($(this).text())">'+streams[i].title+'</a></li>';
            numOffline++;
        }
    }

    if(numOffline == streams.length)
    {
        online += '<li class="item"><div class="online"></div><a style="color:green" href="javascript: void(0)" \
                        title="Random Youtube" onClick="randomTube()">Random Youtube</a></li>';
    }

    $('#list ul').html(curr + online + offline);
}

function editList(){
    clearInterval(updateTimer);
    var online = '';
    var offline = '';
    var curr = '';
    var previousText;

    for(var i = 0; i < streams.length; i++)
    {
        if(i == current && streams[i].status == 'online')
        {
            curr = '<li class="item"><div><img src="greenDelete.png" /></div> \
                        <input type="text" value="'+streams[i].title+'" class="editTitle" /></li>';
        }
        else if(streams[i].status == 'online' && i != curr)
        {
                online += '<li class="item"><div><img src="greenDelete.png" /></div>\
                            <input type="text" value="'+streams[i].title+'" class="editTitle" /></li>';
        }
        else
        {
            offline += '<li class="item"><div><img src="redDelete.png" /></div>\
                            <input type="text" value="'+streams[i].title+'" class="editTitle" /></li>';
        }
    }

    $('#list ul').html(curr + online + offline);

    $(document).on('click', '#list li div img', function(){
        var find = $(this).parent().next("input").val();
        alert(find);
        var index = findObjByTitle(streams, find);
        var obj =  streams.splice(index, 1);
        removeFromStorage(obj[0]);
        $(this).parent().parent().remove();
    });

    $(document).on('focus', '#list input.editTitle', function(){
        previousText = this.value;
    });
    $(document).off('focus', '#list input.editTitle', function(){});

    $(document).on('focusout', '#list input.editTitle', function(){
        var index = findObjByTitle(streams, previousText);
        streams[index].title = this.value;
        updateStorage(streams[index]);
    });
}

function updateViewers(){
    if(typeof(streams[current]) != 'undefined')
        $('#viewers').text(streams[current].viewers);
}

function changeChat(){
    var chatSrc = $('#chatFrame').attr('src');
    var chat = chatSrc.split('channel=');
    $('#chatFrame').attr('src', chat[0]+'channel='+streams[current].url);
    $('#mainChat').removeClass('activeChat');
    $('#streamChat').addClass('activeChat');
}

function changeStream(find){
    var found = false
    $.each(streams, function(index, obj){ 
        $.each(obj, function(key, value){
            if(!found && streams[index].title == find)
            {
                build(index);
                current = index;
                found = true;
                return false;
            }
        });
    });

    if($('#streamChat').hasClass('activeChat'))
        changeChat();
}

function randomTube(){
    var video = tubes[Math.floor(Math.random()*tubes.length)];
    $('#player').html(youtubePlayer);
    $('#player iframe').attr("src", "http://www.youtube.com/embed/"+ video);

    $.ajax({
        url: "http://gdata.youtube.com/feeds/api/videos/"+video+"?v=2&alt=json",
        dataType: "jsonp",
        success: function (data){
            $('#streamInfo').html('<a href="javascript: void(0)" style="color:#ae0000" title="Random Video" onClick="randomTube()">\
                Random Video</a> -- ' + data.entry.title.$t);
        }
    });
    youtube = true;
}

function addChannel(){
    var tempStreamer;
    var title = $('#addChan input[name=title]');
    var url = $('#addChan input[name=url]');

    if(title.val() == '' && url.val() == '')
    {
        title.css("border-color", "red");
        title.val("Enter Streamer Name");
        url.css("border-color", "red");
        url.val("Enter Streamer URL");
        return false;
    }
    else if(title.val() == '')
    {
        title.css("border-color", "red");
        title.val("Enter Streamer Name");
        return false;
    }
    else if(url.val() == '')
    {
        url.css("border-color", "red");
        url.val("Enter Streamer URL");
        return false;
    }
    else
    {
        title = title.val();
        url = (url.serialize()).split('=');
    }
    tempStreamer = new Streamer(title, url[1], null, null);
    $.post(
        "chanExists.php",
        {streams : tempStreamer}, 
        function(data){
            tempStreamer = $.parseJSON(data);
            if(!(tempStreamer.status))
            {
                $('#addChan .successMsg').text("Channel Does Not Exist!");
                $('#addChan .successMsg').css('color', 'red');
            }
            else
            {
                var objIndex = objExists(streams, tempStreamer);

                if(objIndex == -1)
                {
                    $('#addChan .successMsg').text("Channel Added!");
                    $('#addChan .successMsg').css('color', 'green');
                    localStorage.setItem(tempStreamer.url, JSON.stringify(tempStreamer));
                    streams.push(tempStreamer);
                    clearInterval(updateTimer);
                    getList();
                }
                else
                {
                    $('#addChan .successMsg').text("Channel " + streams[objIndex].title + " Already Added");
                    $('#addChan .successMsg').css('color', 'red');
                }
            }
    });
    updateTimer = setInterval(getList, timer);
    return false;
}

/*******************************
/*  The Following are Storage
/*  Manipulation Functions
*******************************/

function getStorage(){
    var tempStream;
    for(var i = 0; i < localStorage.length; i++)
    {
        tempStream = $.parseJSON(localStorage.getItem(localStorage.key(i)));
        streams.push(tempStream);
    }
}

function removeFromStorage(obj){
    var found = false;
    for(var i = 0; (i < localStorage.length && !found); i++)
    {
        tempStream = $.parseJSON(localStorage.getItem(localStorage.key(i)));
        if(tempStream.url = obj.url)
        {
            localStorage.removeItem(obj.url);
            found = true;
        }
    }
}

function updateStorage(obj){
    var found = false;
    for(var i = 0; (i < localStorage.length && !found); i++)
    {
        tempStream = $.parseJSON(localStorage.getItem(localStorage.key(i)));
        if(tempStream.url = obj.url)
        {
            localStorage.removeItem(obj.url);
            localStorage.setItem(obj.url, JSON.stringify(obj));
            found = true;
        }
    }
}

/*******************
/* Streamer Object
/* And Member Functions
*******************/

function Streamer(title, url, status, viewers)
{
    this.title=title;
    this.url=url;
    this.status=status;
    this.viewers=viewers;
}

function objExists(arr, obj)
{
    var i = arr.length;
    while(i--)
    {
        if(arr[i].url == obj.url)
            return i;
    }
    return -1;
}

function findObjByTitle(arr, title)
{
    var found = false;
    var i = 0;
    while(!found)
    {
        if(arr[i].title == title)
        {
            return i;
        }       
        i++;
    }
}
\$\endgroup\$

1 Answer 1

Reset to default
2
\$\begingroup\$

From a once over:

  • You are mixing single double quote and double quote strings, you should stick to 1
  • It is considered better style to have a single comma separated var statement

  • For

    $('#addChan input[name=title]').click(function(){
    var title = $('#addChan input[name=title]');
    if(title.val() == "Enter Streamer Name")
    {
        title.val('');
        title.css("border-color", "");
    }
});
    • You could do `var $this = $(this)
    • You could chain .val and .css to title.val('').css("border-color", "");
    • You might be able to unify $('#addChan input[name=title]').click(function(){ and $('#addChan input[name=url]').click(function(){ they are pretty much the same
  • Your $.post calls have no error handling at all, you should look at that
  • current is a terrible name, current what ?
  • $.each(streams, function(index, obj){ -> obj should have been stream
  • HTML building in general: consider a template function, either find a good library or build your own
  • Your declaration of variables is not consistent, I can understand why you want to declare twitchPlayer on top, because it could get modified and you could consider the top part a 'config section'. But then you declare changeVars inside build().. I think you need to re-think where to put which variables
  • addList has total copy pastage, please consider using a helper function, or perhaps even a temporary variable to hold the <li></li> and then concatenate into the right string
  • The same goes for editList
  • In addChannel you re-purpose title and url from a jQuery object to a value, not good practice
  • In addChannel you should not check for if(title.val() == '' && url.val() == '') as you are basically combining what you already coded for else if(title.val() == '') and else if(url.val() == '')
  • function getStorage(){ is funny and shows a general problem with the code. Instead of return what is in storage, as the name would let you believe, a global variable gets set. On the whole, your code is too reliant on globals
  • In updateStorage, I dont believe you need to call removeItem first
  • I would expect objExists to return a boolean, perhaps call it objIndex ?
  • findObjByTitle
    • You never use found
    • You never return a boolean
    • It seems you could go infinite loop if the title is not at all in the array

I hope my feedback does not sound too harsh, it is still an impressive piece of code. You have extracted a lot of constants into well named variables, your functionality is well split over functions. I think you could just use a few more rounds of polishing.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks for your feedback - Year too late xD - it was a nice stepping stone project and you're right, much of it is redundant and how I handled the ajax call was always sloppy. Not sure what exactly classifies as a answer here but none-the-less I appreciate the input :D ! \$\endgroup\$
    – Howdy_McGee
    Commented Mar 17, 2014 at 20:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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