Counter increment for search volume

I have written a jQuery function which accepts an elementId and an integer. The function then generates a counter which increments at the average speed of searches made (per month) since you initialised the function.

• The elementId is where the counter is placed on the page.

• The integer is the number of monthly searches manually taken from Google adWords.

There are a number of factors I'd like to improve; firstly, the error handling as I am trying to get a reasonably accurate counter Math.round shouldn't be used. But other functions to handle divide by zero issues etc are fine.

I'd also like to improve how the calculations are performed to establish the interval speed, it is currently written to show relatively clear calculation steps (I'd like to think).

```
(function ($) {

/ Initialise Counters/
keywordSearchCounter(9292208, 'footballCounter');
keywordSearchCounter(292410, 'tennisCounter');
keywordSearchCounter(779180, 'chessCounter');

function keywordSearchCounter(searchesPerMonth, elementId) {

if($.type(searchesPerMonth) !== "number") return;

if(searchesPerMonth === 0) return;

console.log("function initialised for '" + elementId.toString() + "'");

/ Do some math on searches /
var daysInMonth = 28,
hoursInDay = 24,
minutesInHour = 60,
secondsInMinute = 60;

var searchesPerDay = searchesPerMonth / daysInMonth;
var searches = searchesPerDay;
console.log('per day: ' + searches);

var searchesPerHour = searchesPerDay / hoursInDay;
searches = searchesPerHour;
console.log('per hour: ' + searches);

var searchesPerMinute = searchesPerHour / minutesInHour;
searches = searchesPerMinute;
console.log('per minute: ' + searches);

var searchesPerSecond = searchesPerMinute / secondsInMinute;
searches = searchesPerSecond;
console.log('per second: ' + searches);

var interval = (1 / searches) * 1000;
var counter = 0;
console.log('interval set to:

I'm going to suggest something completely different to your current implementation... I'll dump the code first (see js fiddle) notice that the fiddle has changed from running the script 'onload' to 'no wrap - in '

// Wrap it all in an IIFE if you want.
var SECONDS_IN_A_MONTH = 60*60*24*30; //seconds * minutes * hours * days

// Search for elements to be turned into counters
$('.search-counter').each(function() {
    var $this, searches, interval, counter;
    $this = $(this);
    searches = parseInt($this.data('searches'), 10);

    if (searches < 1  || isNaN(searches)) return;

    interval = (SECONDS_IN_A_MONTH/searches) 
    counter = 0;

    // Start the counter at zero.
    $this.html(counter);

    setInterval(function () {
        $this.html(++counter);
    }, interval);
});

I've modified your html to include the number of searches per month in a data attribute. I've also added a class to the elements that will be turned into counters. This allows the javascript to deal with any number of counters on a page. E.g. a counter would look like:

 "Chess" has had...  

searches

I don't think you gain anything by splitting up the calculation of the interval. If you think it about another way, you're trying to calculate the time between searches which is clearly just the number of seconds in a month divided by the number of times the search was executed.

If you can't change the html, or would rather keep your code then I have a few points.

• It's nicer to start the counter at zero.

• It's better not to rely on hoisting - put simply, only use a function after you've written it.

• document.getElementById(elementId).innerHTML = counter; will have to search the DOM on every 'tick'. Cache the element

• I used 30 for days in a month rather than 28 - use whichever period the data is from.

All pretty minor stuff really.

StackExchange Code Review Q#71090, answer score: 2