HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Calculate karma based on retweets

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
karmaretweetscalculatebased

Problem

This script is meant to look for an id in the data attribute in each containment div, then send an Ajax call to get the amount of retweets, calculate the karma based on those retweets and then display that number in a div.

This code looks like spaghetti. How do I improve this to make each function more independent? There is some abstraction function that I can't figure out.

My other question is, how can I use a jQuery deferred object to create a promise and update the div once I get a response?

function get_retweet(id) {
    var url = 'https://api.twitter.com/1/statuses/show/' + id + '.json',
        karma; 

    $.ajax({
        url: url,
        type: 'GET',
        dataType:'jsonp',
        crossDomain: true,
        success: function(data) {
            display_karma(data.retweet_count);
        },
        error: function() {alert('fail');}
    }));
}

function calc_karma(tweets) {
    return tweets *10 +10;    
}

function display_karma(retweets) {
    var id, karma,
        el =$('.tweetContainer');
    //id = $(el).data(el, tweet_id);
    id = '248988915661410304';
    karma = calc_karma(retweets);
    el.find('.tcPoints').text(karma);
}

function start_get_karma() {
    var id;

    $('.tweetContainer').each(function(index,el) {
        //id = $(el).data(el, tweet_id);
        id = '248988915661410304';
        get_retweet(id);
    });
}

Solution

Here are some tips:

-
Use $.getJSON() to get JSON.

Also, name the first parameter json for clarity.

Note from documentation for jQuery.getJSON()


JSONP


If the URL includes the string "callback=?" (or similar, as defined by
the server-side API), the request is treated as JSONP instead. See the
discussion of the jsonp data type in $.ajax() for more details.

More information here

Old Code:

var url = 'https://api.twitter.com/1/statuses/show/' + id + '.json',
    karma; 
$.ajax({
    url: url,
    type: 'GET',
    dataType:'jsonp',
    crossDomain: true,
    success: function(data) {
        display_karma(data.retweet_count);
    },
    error: function() {alert('fail');}
}));


New Code:

var url = 'https://api.twitter.com/1/statuses/show/' + id + '.json?callback=?';
$.getJSON( url, function (json) {
    display_karma(json.retweet_count);
}).error(function () {
    alert('fail');
});


-
Eliminate commented code. Use a source control system like git, svn, mercurial to keep track of changes.

Here's a good start for it

Old Code:

function start_get_karma() {
    var id;

    $('.tweetContainer').each(function(index,el) {
        //id = $(el).data(el, tweet_id);
        id = '248988915661410304';
        get_retweet(id);
    });
}


New Code:

function start_get_karma() {
    var id = '248988915661410304';
    $('.tweetContainer').each(function (index, el) {
        get_retweet(id);
    });
}


-
Function calls are expensive, so use a basic loop instead of each() when appropriate.

Old Code:

$('.tweetContainer').each(function (index, el) {
    get_retweet(id);
});


New Code A:

for(var i = 0; len = $('.tweetContainer').length; i < len; i++){
    get_retweet(id);
}


However, there's a problem. Making multiple calls to get_retweet() with the same static value doesn't make sense. So just make one call.

New Code B:

if( $('.tweetContainer').length ){
    get_retweet(id);
}


-
Don't declare variables if they're only used once.

Old Code:

function display_karma(retweets) {
    var id, karma,
        el =$('.tweetContainer');
        id = '248988915661410304';
    karma = calc_karma(retweets);
    el.find('.tcPoints').text(karma);
}


New Code:

function display_karma(retweets) {
    var id = '248988915661410304';
    $('.tweetContainer').find('.tcPoints').text(calc_karma(retweets));
}


-
Have variable names give a hint to the type.

retweets sounds like a function or array. Try naming it retweet_amount or something similar.

Final Code:

function calc_karma(tweets) {
    return (tweets * 10) + 10;
}
function display_karma(retweet_amount) {
    $('.tweetContainer').find('.tcPoints').text(calc_karma(retweet_amount));
}
function get_retweet(id) {
    var url = 'https://api.twitter.com/1/statuses/show/' + id + '.json?callback=?';
    $.getJSON( url, function (json) {
        display_karma(json.retweet_count);
    }).error(function () {
        alert('fail');
    });
}
function start_get_karma() {
    var id = '248988915661410304';
    if( $('.tweetContainer').length ){
        get_retweet(id);
    }
}

Code Snippets

var url = 'https://api.twitter.com/1/statuses/show/' + id + '.json',
    karma; 
$.ajax({
    url: url,
    type: 'GET',
    dataType:'jsonp',
    crossDomain: true,
    success: function(data) {
        display_karma(data.retweet_count);
    },
    error: function() {alert('fail');}
}));
var url = 'https://api.twitter.com/1/statuses/show/' + id + '.json?callback=?';
$.getJSON( url, function (json) {
    display_karma(json.retweet_count);
}).error(function () {
    alert('fail');
});
function start_get_karma() {
    var id;

    $('.tweetContainer').each(function(index,el) {
        //id = $(el).data(el, tweet_id);
        id = '248988915661410304';
        get_retweet(id);
    });
}
function start_get_karma() {
    var id = '248988915661410304';
    $('.tweetContainer').each(function (index, el) {
        get_retweet(id);
    });
}
$('.tweetContainer').each(function (index, el) {
    get_retweet(id);
});

Context

StackExchange Code Review Q#15860, answer score: 9

Revisions (0)

No revisions yet.