CODEWITHSUNDEEP
javascriptjquery
purpleoctopus
purpleoctopus

Reputation: 107

Code suggestions - using $.each() to set a kind of use meter

Hoping for a quick peer review here. An associate and I build out a video popchart plugin for a client in south korea (here's the test site - http://izepic.com/kpopcharts/). My question relates to the activity meter in the header of each vid player. So, I wrote the js below to check each of the social interaction numbers, determine their percentage of the interaction total, and then set the width of each type in the meter itself. Note, specificity for each interaction type was required.

$('.bkp-meter').each(function(index){

// find participation type base numbers
var voteTotal = parseInt($('.bkp-vote-total').eq(index).text());
var facebookTotal = parseInt($('.bkp-facebook-total').eq(index).text());
var twitterTotal = parseInt($('.bkp-twitter-total').eq(index).text());
var googleTotal = parseInt($('.bkp-google-total').eq(index).text());
var commentTotal = parseInt($('.bkp-comment-total').eq(index).text());

var scoreTotal = voteTotal + facebookTotal + twitterTotal + googleTotal + commentTotal;

// find participation type ratio
var votePercentage = (voteTotal / scoreTotal) * 100;
var facebookPercentage = (facebookTotal / scoreTotal) * 100;
var twitterPercentage = (twitterTotal / scoreTotal) * 100;
var googlePercentage = (googleTotal / scoreTotal) * 100;
var commentPercentage = (commentTotal / scoreTotal) * 100;

if(scoreTotal > 2) {

// set meter widths for each participation type
$('.bkp-meter-votes').eq(index).css('width', (votePercentage.toFixed(0) - 2) + "%");
$('.bkp-meter-fb').eq(index).css('width',facebookPercentage.toFixed(0) + "%");
$('.bkp-meter-twitter').eq(index).css('width',twitterPercentage.toFixed(0) + "%");
$('.bkp-meter-google').eq(index).css('width',googlePercentage.toFixed(0) + "%");
$('.bkp-meter-comments').eq(index).css('width',(commentPercentage.toFixed(0)) + "%");

} else {
$(this).parent().parent().addClass('novotes');
}

});

My question: is there a quicker, cleaner way to do this? I mean, it's working fine so problem solved but it feels very brute force ... for my own improvement I'd like to know if there's a more efficient method and what sort of issues I might run into with this. One note - the percentages didn't have to be perfect ... they just need to give the user a quick shot of what other people are doin' with that vid.

Thanks

Upvotes: 0

Views: 90

Answers (2)

Eric
Eric

Reputation: 97571

As a programmer, you should generalize when there is more than one thing:

var participationTypes = ['vote', 'facebook', 'twitter', 'google', 'comment'];

$('.bkp-meter').each(function() {
    var meter = $(this);

    var scores = {};
    var scoreTotal = 0;

    $.each(participationTypes, function() {
        scores[this] = parseInt(meter.find('.bkp-'+this+'-total').text());
        scoreTotal += scores[this];
    });

    if(scoreTotal > 2)
        $.each(participationTypes, function() {
            meter.find('.bkp-meter-'+this).width(
                Math.round(scores[this] / scoreTotal * 100) + '%'
            );
        });
    else
        meter.parent().parent().addClass('novotes');
});

Upvotes: 1

Adam Terlson
Adam Terlson

Reputation: 12730

$.each() returns two params, one is index and the other is the object itself. You can optimize your selectors by only searching within that context.

$('.bkp-meter').each(function(index, meter){
...

$('.bkp-meter-votes', meter).css('width', (votePercentage.toFixed(0) - 2) + "%");
$('.bkp-meter-fb', meter).css('width',facebookPercentage.toFixed(0) + "%");

...

});

Same thing applies to your first block with the $('.bkp-vote-total').eq(index) bits.

Also note that class selectors are really slow in browsers that don't support getElementByClass natively (namely IE8 and lower) so beware using them alone like that if it's a concern. Always provide context: $('#container .bkp-meter')

Upvotes: 0

Related Questions