patternjavascriptMinor
Calculate karma based on retweets
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?
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
Also, name the first parameter
Note from documentation for
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:
New Code:
-
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:
New Code:
-
Function calls are expensive, so use a basic loop instead of
Old Code:
New Code A:
However, there's a problem. Making multiple calls to
New Code B:
-
Don't declare variables if they're only used once.
Old Code:
New Code:
-
Have variable names give a hint to the type.
Final Code:
-
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.